Skip to content
This repository has been archived by the owner on Sep 27, 2019. It is now read-only.

[15721] TileGroup Compaction and GC Fixes #1349

Open
wants to merge 121 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
121 commits
Select commit Hold shift + click to select a range
53d21cf
Created GC CommitDelete Test. Made ClearGarbage public in TLGC
Apr 19, 2018
5cf3a0f
Modified CommitDelete test so that it properly cleans up the database
Apr 20, 2018
5a9b0ef
Added 14 tests to transaction-level GC manager. Captures 4 GC failures.
Apr 20, 2018
8800842
Added test utilities, and multiple new test cases for checking
Apr 21, 2018
729f96e
Added more index tests. Added tests for primary key updates.
Apr 21, 2018
0d93313
Added PrimaryKeyUpdateTest
mbutrovich Apr 21, 2018
ded93a7
Refactor.
mbutrovich Apr 21, 2018
469ae2c
Fixed eror in CommitUpdateSecondaryKeyTest.
mbutrovich Apr 22, 2018
39e0727
Fixed bug where tuple slots are not reclaimed when insertions fail.
Apr 22, 2018
1be98a5
Updated transaction manager and garbage collector to properly handle
Apr 22, 2018
0750a84
Updated GetRecycledTupleSlot() in GC to no longer hand out slots from
Apr 22, 2018
3f592d5
Enhanced GC tests to check indexes in all cases, uses new test function
Apr 23, 2018
1242ec3
Modified the GC so that it properly removes index entries for
Apr 23, 2018
22087ac
Minor refactor and comments before a formatting run.
mbutrovich Apr 23, 2018
84efd9e
clang-format-3.6 on modified files.
mbutrovich Apr 23, 2018
8f9f25a
Created GC CommitDelete Test. Made ClearGarbage public in TLGC
Apr 19, 2018
c07fb50
Modified CommitDelete test so that it properly cleans up the database
Apr 20, 2018
2eb08c2
Added 14 tests to transaction-level GC manager. Captures 4 GC failures.
Apr 20, 2018
a1dbd5a
Added test utilities, and multiple new test cases for checking
Apr 21, 2018
118008b
Added more index tests. Added tests for primary key updates.
Apr 21, 2018
cd01623
Added PrimaryKeyUpdateTest
mbutrovich Apr 21, 2018
b81817c
Refactor.
mbutrovich Apr 21, 2018
d6d90cc
Fixed bug where tuple slots are not reclaimed when insertions fail.
Apr 22, 2018
04353ad
Updated transaction manager and garbage collector to properly handle
Apr 22, 2018
21c7874
Enhanced GC tests to check indexes in all cases, uses new test function
Apr 23, 2018
01db881
Modified the GC so that it properly removes index entries for
Apr 23, 2018
ad604a2
Minor refactor and comments before a formatting run.
mbutrovich Apr 23, 2018
e35d818
clang-format-3.6 on modified files.
mbutrovich Apr 23, 2018
7b3218b
Disabled 4 failing tests that are not addressed in this PR and will o…
mbutrovich Apr 23, 2018
a2baf6a
Reenabled 4 disabled tests because we still want to test recycle slot…
mbutrovich Apr 23, 2018
c45adff
Fixed unused attribute
poojanilangekar Apr 23, 2018
f79660c
Revert "clang-format-3.6 on modified files."
poojanilangekar Apr 24, 2018
925e3c1
Revert "clang-format-3.6 again after rebase."
poojanilangekar Apr 24, 2018
62521cb
Revert some remaining format changes + disable tests
poojanilangekar Apr 24, 2018
a3c2d56
Applied all TileGroup compaction changes to the GC Fixes branch.
May 4, 2018
112ec8c
Created GC CommitDelete Test. Made ClearGarbage public in TLGC
Apr 19, 2018
87281e3
Modified CommitDelete test so that it properly cleans up the database
Apr 20, 2018
3da3d05
Added 14 tests to transaction-level GC manager. Captures 4 GC failures.
Apr 20, 2018
4777dc8
Added test utilities, and multiple new test cases for checking
Apr 21, 2018
8f1c840
Added more index tests. Added tests for primary key updates.
Apr 21, 2018
f5e9ed0
Added PrimaryKeyUpdateTest
mbutrovich Apr 21, 2018
da2879b
Refactor.
mbutrovich Apr 21, 2018
6331fd4
Fixed bug where tuple slots are not reclaimed when insertions fail.
Apr 22, 2018
f83be68
Enhanced GC tests to check indexes in all cases, uses new test function
Apr 23, 2018
238abed
Minor refactor and comments before a formatting run.
mbutrovich Apr 23, 2018
e77979d
clang-format-3.6 on modified files.
mbutrovich Apr 23, 2018
9f1dd1d
Created GC CommitDelete Test. Made ClearGarbage public in TLGC
Apr 19, 2018
18f9de2
Modified CommitDelete test so that it properly cleans up the database
Apr 20, 2018
4de37d0
Added 14 tests to transaction-level GC manager. Captures 4 GC failures.
Apr 20, 2018
a710ed0
Added test utilities, and multiple new test cases for checking
Apr 21, 2018
04a03ab
Added more index tests. Added tests for primary key updates.
Apr 21, 2018
3d812b1
Added PrimaryKeyUpdateTest
mbutrovich Apr 21, 2018
cb00215
Refactor.
mbutrovich Apr 21, 2018
6508ddb
Fixed eror in CommitUpdateSecondaryKeyTest.
mbutrovich Apr 22, 2018
901fd61
Fixed bug where tuple slots are not reclaimed when insertions fail.
Apr 22, 2018
88c4ac2
Updated transaction manager and garbage collector to properly handle
Apr 22, 2018
f743bd7
Updated GetRecycledTupleSlot() in GC to no longer hand out slots from
Apr 22, 2018
7d67573
Enhanced GC tests to check indexes in all cases, uses new test function
Apr 23, 2018
b78607a
Minor refactor and comments before a formatting run.
mbutrovich Apr 23, 2018
652d200
clang-format-3.6 on modified files.
mbutrovich Apr 23, 2018
4bb69e9
clang-format-3.6 again after rebase.
mbutrovich Apr 23, 2018
94d6245
Disabled 4 failing tests that are not addressed in this PR and will o…
mbutrovich Apr 23, 2018
1373bb2
Reenabled 4 disabled tests because we still want to test recycle slot…
mbutrovich Apr 23, 2018
624195a
Revert "clang-format-3.6 on modified files."
poojanilangekar Apr 24, 2018
1135d45
Revert "clang-format-3.6 again after rebase."
poojanilangekar Apr 24, 2018
f68e182
Revert some remaining format changes + disable tests
poojanilangekar Apr 24, 2018
f32b079
Applied all TileGroup compaction changes to the GC Fixes branch.
May 4, 2018
5663fac
Minor fixes after rebase.
mbutrovich May 5, 2018
680b30d
More minor fixes after rebase.
mbutrovich May 5, 2018
ce044fa
Fixed table naming to be lowercase to fix exception in table generation.
mbutrovich May 5, 2018
a8459c5
Fixed errant copy-paste.
mbutrovich May 5, 2018
c2a0564
Removed reliance on GC Manager's tables_ structure. Go to the storage…
mbutrovich May 5, 2018
d69c302
Changed tile group freeing approach. Going to use lock-free stack ins…
May 8, 2018
ad00934
Finished initial draft of GCManager. Renamed and refactored UnlinkVer…
May 8, 2018
405f32f
Created RecycleStack class. Not yet tested.
May 9, 2018
35b856d
Bug fixes, compiles, runs transaction_level_gc_manager_tests.
mbutrovich May 9, 2018
5210f0f
Minor bug fixes. Passes make check. Need to write more tests to execr…
mbutrovich May 9, 2018
ed222a0
Added a way to lookup elements in LockFreeArray. Need this to do a re…
mbutrovich May 9, 2018
35ded84
Preliminary implementation of TileGroup compaction. Needs to be tested.
May 10, 2018
698e811
Bug fix in compaction (removed LogicalTile), and added a new test for…
mbutrovich May 10, 2018
92f53fa
Made immutable_tile_group_queue a single queue instead of multiple, m…
mbutrovich May 10, 2018
9ca16c0
Moved TileGroup compaction to a stand-alone class, separate from the GC.
mbutrovich May 10, 2018
14327c8
Modifications to handle removing TileGroups: made manager.h actually …
mbutrovich May 11, 2018
d058b17
2 small changes based on PR comments.
mbutrovich May 11, 2018
c962de5
Refactor to Manager class for TileGroup dropping.
mbutrovich May 11, 2018
aadb61e
Fixed typo in manager.cpp.
mbutrovich May 11, 2018
a1f2f2f
Refactor and comments regarding immutability.
mbutrovich May 11, 2018
d714595
Reenabled old GC tests. Modified immutability test to be correct with…
mbutrovich May 11, 2018
b43951b
Refactor of TransactionLevelGCManager. Added compaction queue.
May 11, 2018
bfd38ac
Audited codebase to find all places where tile_groups dereferenced
May 12, 2018
10255fe
Created TileGroupCompactor concurrency test.
May 12, 2018
32088a0
Added several TileGroupCompactor tests
May 12, 2018
26b830a
Fixed two possible nullptr dereferences in TOTM, added some LOG_TRACE…
mbutrovich May 13, 2018
5ea8fa5
Final pass of comments. Added Setter for GC.compaction_threshold_.
May 14, 2018
5cbece2
Added global setting for TileGroup freeing, default is off.
mbutrovich May 21, 2018
557364a
RecycleStack Doxygen comments.
mbutrovich May 21, 2018
e9ca79e
TileGroupCompactor Doxygen comments.
mbutrovich May 21, 2018
4d879da
TransactionLevelGCManager Doxygen comments.
mbutrovich May 21, 2018
775518e
Minor refactor and clang-tidy changes.
mbutrovich May 22, 2018
982f8a8
More minor refactors and changes based on PR feedback.
mbutrovich May 22, 2018
448e307
Added a boolean to disable compaction entirely.
mbutrovich May 22, 2018
1988155
Putting back the TileGroupIterator for future enhancement.
mbutrovich May 22, 2018
77d49a9
Fixed TRACE-level logging issues in GC manager, added some Doxygen co…
mbutrovich May 22, 2018
7b6edbe
Post-rebase fixes.
mbutrovich May 22, 2018
4e54284
Fixed bug in manager.cpp after rebase. Removed unnecessary variables …
mbutrovich May 23, 2018
785f2e9
clang-format-3.6 on modified lines.
mbutrovich May 24, 2018
d284122
Fix formatting issue on recycle_stack's include.
mbutrovich May 24, 2018
c09e684
Fix unused variables when tests compiled in Release mode.
mbutrovich May 24, 2018
6014375
Fix printing issue in tile_group_compactor_test and reduced LOG_DEBUG…
mbutrovich May 24, 2018
a7e99c2
Fix unused variables in Release mode.
mbutrovich May 25, 2018
a3708d9
Fix Debug printing issue that last commit introduced.
mbutrovich May 25, 2018
4a921f6
Added IsRunning member to MonoQueuePool so TileGroupCompactor can che…
mbutrovich May 25, 2018
aaefb0f
Fix include issue for GCC on Travis.
mbutrovich May 29, 2018
e18919d
Fix memory ordering issue for GCC on Travis.
mbutrovich May 29, 2018
5a45684
Added more comments to disabled tests, fixed typo in tests.
mbutrovich May 30, 2018
fbb27de
New test scenario that triggers an assertion fail in TOTM (CommitInse…
mbutrovich Jun 1, 2018
9e1d943
Post-rebase fixes.
mbutrovich Jun 4, 2018
7797be2
More post-rebase fixes. Passes make check again.
mbutrovich Jun 4, 2018
ac86a4e
Fix condition in tile_group_compactor test that was too strict for so…
mbutrovich Jun 12, 2018
9a9477a
Minor refactor and comment changes.
mbutrovich Jun 13, 2018
002881b
Fix tile_group_compactor not recording ReadOwn behavior.
mbutrovich Jun 18, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/catalog/manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,4 @@ void Manager::DropIndirectionArray(const oid_t oid) {
void Manager::ClearIndirectionArray() { indirection_array_locator_.clear(); }

} // namespace catalog
} // namespace peloton
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style change, please remove it.

} // namespace peloton
1 change: 1 addition & 0 deletions src/codegen/deleter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ void Deleter::Delete(uint32_t tile_group_id, uint32_t tuple_offset) {

auto *txn = executor_context_->GetTransaction();
auto tile_group = table_->GetTileGroupById(tile_group_id);
PELOTON_ASSERT(tile_group != nullptr);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this line added? Is this necessary? Or was it for debugging?
Can it cause undefined behavior in Release mode?

Copy link
Contributor

@mbutrovich mbutrovich Jun 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetTileGroupById can now return a nullptr if the TileGroup has been dropped, this is just a check that we don't hit that during make check and other Debug builds. That's what most of the ASSERTs added to the execution layer are looking for.

If you're suggesting that we make it impossible to return a nullptr to the execution level, then we need to revisit how we access TileGroups and go only through an iterator that will always be valid. That may in fact be the correct solution, but is a larger design decision that I'd have to loop in people more familiar with what the execution layer (particularly codegen going forward) would want out of it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need to touch the execution layer. In a correct implementation, the execution layer should be completely oblivious to the GC.

GetTileGroupById can now return a nullptr if the TileGroup has been dropped. -> That is my biggest concern. So this thing is checked in debug which is great. But what happens in release mode? If this function gets a nullptr, it hits a segfault and peloton crashes? That's not expected behavior.

Like I said, in my overall comment, the whole point of the epoch based garbage collection is that the TileGroup references are removed only when you are certain that no other transaction is accessing it. This is very similar to GC in the SkipListIndex. The storage may get these nullptr references, but the execution layer should never get such a reference.

I would suggest that we can either do a hangouts call over the weekend. Or you talk to @apavlo about this.

auto *tile_group_header = tile_group->GetHeader();

auto &txn_manager = concurrency::TransactionManagerFactory::GetInstance();
Expand Down
9 changes: 7 additions & 2 deletions src/codegen/inserter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ void Inserter::Init(storage::DataTable *table,
char *Inserter::AllocateTupleStorage() {
location_ = table_->GetEmptyTupleSlot(nullptr);

PELOTON_ASSERT(location_.IsNull() == false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question as above. Why was this added? Ideally, I believe these checks should be handled at a higher layer, probably the DataTable.


// Get the tile offset assuming that it is a row store
auto tile_group = table_->GetTileGroupById(location_.block);
auto layout = tile_group->GetLayout();
Expand All @@ -54,8 +56,11 @@ void Inserter::Insert() {
auto *txn = executor_context_->GetTransaction();
auto &txn_manager = concurrency::TransactionManagerFactory::GetInstance();

ContainerTuple<storage::TileGroup> tuple(
table_->GetTileGroupById(location_.block).get(), location_.offset);
auto tile_group = table_->GetTileGroupById(location_.block).get();
PELOTON_ASSERT(tile_group != nullptr);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question. Why was this added?


ContainerTuple<storage::TileGroup> tuple(tile_group, location_.offset);

ItemPointer *index_entry_ptr = nullptr;
bool result = table_->InsertTuple(&tuple, location_, txn, &index_entry_ptr);
if (result == false) {
Expand Down
10 changes: 8 additions & 2 deletions src/codegen/updater.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ char *Updater::Prepare(uint32_t tile_group_id, uint32_t tuple_offset) {

auto *txn = executor_context_->GetTransaction();
auto tile_group = table_->GetTileGroupById(tile_group_id).get();
PELOTON_ASSERT(tile_group != nullptr);
auto *tile_group_header = tile_group->GetHeader();
old_location_.block = tile_group_id;
old_location_.offset = tuple_offset;
Expand Down Expand Up @@ -91,6 +92,7 @@ char *Updater::PreparePK(uint32_t tile_group_id, uint32_t tuple_offset) {

auto *txn = executor_context_->GetTransaction();
auto tile_group = table_->GetTileGroupById(tile_group_id).get();
PELOTON_ASSERT(tile_group != nullptr);
auto *tile_group_header = tile_group->GetHeader();

// Check ownership
Expand Down Expand Up @@ -135,6 +137,7 @@ void Updater::Update() {
table_->GetOid());
auto *txn = executor_context_->GetTransaction();
auto tile_group = table_->GetTileGroupById(old_location_.block).get();
PELOTON_ASSERT(tile_group != nullptr);
auto *tile_group_header = tile_group->GetHeader();
auto &txn_manager = concurrency::TransactionManagerFactory::GetInstance();
// Either update in-place
Expand All @@ -147,8 +150,10 @@ void Updater::Update() {
}

// Or, update with a new version
ContainerTuple<storage::TileGroup> new_tuple(
table_->GetTileGroupById(new_location_.block).get(), new_location_.offset);
auto new_tile_group = table_->GetTileGroupById(new_location_.block);
PELOTON_ASSERT(new_tile_group != nullptr);
ContainerTuple<storage::TileGroup> new_tuple(new_tile_group.get(),
new_location_.offset);
ItemPointer *indirection =
tile_group_header->GetIndirection(old_location_.offset);
auto result = table_->InstallVersion(&new_tuple, target_list_, txn,
Expand All @@ -171,6 +176,7 @@ void Updater::UpdatePK() {
table_->GetOid());
auto *txn = executor_context_->GetTransaction();
auto tile_group = table_->GetTileGroupById(new_location_.block).get();
PELOTON_ASSERT(tile_group != nullptr);
auto &txn_manager = concurrency::TransactionManagerFactory::GetInstance();

// Insert a new tuple
Expand Down
6 changes: 6 additions & 0 deletions src/common/container/cuckoo_map.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
#include "common/item_pointer.h"
#include "common/logger.h"
#include "common/macros.h"
#include "common/container/lock_free_queue.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need to include this file and storage/data_table.h.

#include "gc/recycle_stack.h"
#include "storage/data_table.h"

namespace peloton {

Expand Down Expand Up @@ -125,4 +128,7 @@ template class CuckooMap<std::shared_ptr<oid_t>, std::shared_ptr<oid_t>>;
// Used in StatementCacheManager
template class CuckooMap<StatementCache *, StatementCache *>;

// Used in TransactionLevelGCManager
template class CuckooMap<oid_t, std::shared_ptr<gc::RecycleStack>>;

} // namespace peloton
15 changes: 15 additions & 0 deletions src/common/container/lock_free_array.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ template <typename ValueType>
void LOCK_FREE_ARRAY_TYPE::Erase(const std::size_t &offset,
const ValueType &invalid_value) {
LOG_TRACE("Erase at %lu", offset);
PELOTON_ASSERT(lock_free_array.size() > offset);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, was this for debugging? Or have you introduced some new logic that could cause this condition?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No new logic. .at() will throw an exception that we never catch if it's out of bounds. Seems more correct to me to have a PELOTON_ASSERT first. Also, see Find() and Update() which already have that logic. Erase() not having it just looks like an oversight.

lock_free_array.at(offset) = invalid_value;
}

Expand Down Expand Up @@ -107,6 +108,20 @@ bool LOCK_FREE_ARRAY_TYPE::Contains(const ValueType &value) const {
return exists;
}

template <typename ValueType>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two main concerns:

  1. What happens if another thread modifies the value between line 115 and 117?
  2. This seems inefficient. What is the use case? Would using a CuckooHash be more efficient?

ssize_t LOCK_FREE_ARRAY_TYPE::Lookup(const ValueType &value) {
for (std::size_t array_itr = 0; array_itr < lock_free_array.size();
array_itr++) {
auto array_value = lock_free_array.at(array_itr);
// Check array value
if (array_value == value) {
return array_itr;
}
}

return -1;
}

// Explicit template instantiation
template class LockFreeArray<std::shared_ptr<oid_t>>;

Expand Down
2 changes: 1 addition & 1 deletion src/common/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ void PelotonInit::Initialize() {
threadpool::MonoQueuePool::GetExecutionInstance().Startup();

int parallelism = (CONNECTION_THREAD_COUNT + 3) / 4;
storage::DataTable::SetActiveTileGroupCount(parallelism);
storage::DataTable::SetDefaultActiveTileGroupCount(parallelism);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job!

storage::DataTable::SetActiveIndirectionArrayCount(parallelism);

// start epoch.
Expand Down
8 changes: 4 additions & 4 deletions src/common/internal_types.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2967,8 +2967,8 @@ std::string GCVersionTypeToString(GCVersionType type) {
case GCVersionType::ABORT_UPDATE: {
return "ABORT_UPDATE";
}
case GCVersionType::ABORT_DELETE: {
return "ABORT_DELETE";
case GCVersionType::TOMBSTONE: {
return "TOMBSTONE";
}
case GCVersionType::ABORT_INSERT: {
return "ABORT_INSERT";
Expand Down Expand Up @@ -2997,8 +2997,8 @@ GCVersionType StringToGCVersionType(const std::string &str) {
return GCVersionType::COMMIT_INS_DEL;
} else if (upper_str == "ABORT_UPDATE") {
return GCVersionType::ABORT_UPDATE;
} else if (upper_str == "ABORT_DELETE") {
return GCVersionType::ABORT_DELETE;
} else if (upper_str == "TOMBSTONE") {
return GCVersionType::TOMBSTONE;
} else if (upper_str == "ABORT_INSERT") {
return GCVersionType::ABORT_INSERT;
} else if (upper_str == "ABORT_INS_DEL") {
Expand Down
30 changes: 20 additions & 10 deletions src/concurrency/timestamp_ordering_transaction_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -290,9 +290,9 @@ bool TimestampOrderingTransactionManager::PerformRead(TransactionContext *const
//////////////////////////////////////////////////////////
else {
PELOTON_ASSERT(current_txn->GetIsolationLevel() ==
IsolationLevelType::SERIALIZABLE ||
current_txn->GetIsolationLevel() ==
IsolationLevelType::REPEATABLE_READS);
IsolationLevelType::SERIALIZABLE ||
current_txn->GetIsolationLevel() ==
IsolationLevelType::REPEATABLE_READS);

oid_t tuple_id = location.offset;

Expand Down Expand Up @@ -580,6 +580,9 @@ void TimestampOrderingTransactionManager::PerformDelete(
current_txn->RecordDelete(old_location);
}

// Performs Delete on a tuple that was created by the current transaction, and
// never
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Combine line 584 and 585.

// installed into the database
void TimestampOrderingTransactionManager::PerformDelete(
TransactionContext *const current_txn, const ItemPointer &location) {
PELOTON_ASSERT(!current_txn->IsReadOnly());
Expand Down Expand Up @@ -737,6 +740,9 @@ ResultType TimestampOrderingTransactionManager::CommitTransaction(
gc_set->operator[](tile_group_id)[tuple_slot] =
GCVersionType::COMMIT_DELETE;

gc_set->operator[](new_version.block)[new_version.offset] =
GCVersionType::TOMBSTONE;

log_manager.LogDelete(ItemPointer(tile_group_id, tuple_slot));

} else if (tuple_entry.second == RWType::INSERT) {
Expand Down Expand Up @@ -855,9 +861,11 @@ ResultType TimestampOrderingTransactionManager::AbortTransaction(
// before we unlink the aborted version from version list
ItemPointer *index_entry_ptr =
tile_group_header->GetIndirection(tuple_slot);
UNUSED_ATTRIBUTE auto res = AtomicUpdateItemPointer(
index_entry_ptr, ItemPointer(tile_group_id, tuple_slot));
PELOTON_ASSERT(res == true);
if (index_entry_ptr) {
UNUSED_ATTRIBUTE auto res = AtomicUpdateItemPointer(
index_entry_ptr, ItemPointer(tile_group_id, tuple_slot));
PELOTON_ASSERT(res == true);
}
//////////////////////////////////////////////////

// we should set the version before releasing the lock.
Expand Down Expand Up @@ -903,9 +911,11 @@ ResultType TimestampOrderingTransactionManager::AbortTransaction(
// before we unlink the aborted version from version list
ItemPointer *index_entry_ptr =
tile_group_header->GetIndirection(tuple_slot);
UNUSED_ATTRIBUTE auto res = AtomicUpdateItemPointer(
index_entry_ptr, ItemPointer(tile_group_id, tuple_slot));
PELOTON_ASSERT(res == true);
if (index_entry_ptr) {
UNUSED_ATTRIBUTE auto res = AtomicUpdateItemPointer(
index_entry_ptr, ItemPointer(tile_group_id, tuple_slot));
PELOTON_ASSERT(res == true);
}
//////////////////////////////////////////////////

// we should set the version before releasing the lock.
Expand All @@ -923,7 +933,7 @@ ResultType TimestampOrderingTransactionManager::AbortTransaction(

// add the version to gc set.
gc_set->operator[](new_version.block)[new_version.offset] =
GCVersionType::ABORT_DELETE;
GCVersionType::TOMBSTONE;

} else if (tuple_entry.second == RWType::INSERT) {
tile_group_header->SetBeginCommitId(tuple_slot, MAX_CID);
Expand Down
16 changes: 16 additions & 0 deletions src/executor/hybrid_scan_executor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,9 @@ bool HybridScanExecutor::DInit() {
tile_group = table_->GetTileGroup(table_tile_group_count_ - 1);
}

// TODO: Handle possibility of freed tile_group
PELOTON_ASSERT(tile_group != nullptr);

oid_t tuple_id = 0;
ItemPointer location(tile_group->GetTileGroupId(), tuple_id);
block_threshold = location.block;
Expand Down Expand Up @@ -189,6 +192,10 @@ bool HybridScanExecutor::SeqScanUtil() {
while (current_tile_group_offset_ < table_tile_group_count_) {
LOG_TRACE("Current tile group offset : %u", current_tile_group_offset_);
auto tile_group = table_->GetTileGroup(current_tile_group_offset_++);
if (tile_group == nullptr) {
continue;
}

auto tile_group_header = tile_group->GetHeader();

oid_t active_tuple_count = tile_group->GetNextTupleSlot();
Expand Down Expand Up @@ -380,6 +387,9 @@ bool HybridScanExecutor::ExecPrimaryIndexLookup() {

auto storage_manager = storage::StorageManager::GetInstance();
auto tile_group = storage_manager->GetTileGroup(tuple_location.block);

// TODO: Handle possibility of freed tile_group
PELOTON_ASSERT(tile_group != nullptr);
auto tile_group_header = tile_group.get()->GetHeader();

// perform transaction read
Expand Down Expand Up @@ -427,6 +437,9 @@ bool HybridScanExecutor::ExecPrimaryIndexLookup() {
}

tile_group = storage_manager->GetTileGroup(tuple_location.block);

// TODO: Handle possibility of freed tile_group
PELOTON_ASSERT(tile_group != nullptr);
tile_group_header = tile_group.get()->GetHeader();
}
}
Expand All @@ -437,6 +450,9 @@ bool HybridScanExecutor::ExecPrimaryIndexLookup() {
auto storage_manager = storage::StorageManager::GetInstance();
auto tile_group = storage_manager->GetTileGroup(tuples.first);

// TODO: Handle possibility of freed tile_group
PELOTON_ASSERT(tile_group != nullptr);

std::unique_ptr<LogicalTile> logical_tile(LogicalTileFactory::GetTile());

// Add relevant columns to logical tile
Expand Down
11 changes: 11 additions & 0 deletions src/executor/index_scan_executor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ bool IndexScanExecutor::ExecPrimaryIndexLookup() {
for (auto tuple_location_ptr : tuple_location_ptrs) {
ItemPointer tuple_location = *tuple_location_ptr;
auto tile_group = storage_manager->GetTileGroup(tuple_location.block);
PELOTON_ASSERT(tile_group != nullptr);
auto tile_group_header = tile_group.get()->GetHeader();
size_t chain_length = 0;

Expand Down Expand Up @@ -294,6 +295,7 @@ bool IndexScanExecutor::ExecPrimaryIndexLookup() {
*(tile_group_header->GetIndirection(tuple_location.offset));
auto storage_manager = storage::StorageManager::GetInstance();
tile_group = storage_manager->GetTileGroup(tuple_location.block);
PELOTON_ASSERT(tile_group != nullptr);
tile_group_header = tile_group.get()->GetHeader();
chain_length = 0;
continue;
Expand All @@ -320,6 +322,7 @@ bool IndexScanExecutor::ExecPrimaryIndexLookup() {
// search for next version.
auto storage_manager = storage::StorageManager::GetInstance();
tile_group = storage_manager->GetTileGroup(tuple_location.block);
PELOTON_ASSERT(tile_group != nullptr);
tile_group_header = tile_group.get()->GetHeader();
continue;
}
Expand Down Expand Up @@ -355,6 +358,7 @@ bool IndexScanExecutor::ExecPrimaryIndexLookup() {
// into the result vector
auto storage_manager = storage::StorageManager::GetInstance();
auto tile_group = storage_manager->GetTileGroup(current_tile_group_oid);
PELOTON_ASSERT(tile_group != nullptr);
std::unique_ptr<LogicalTile> logical_tile(LogicalTileFactory::GetTile());
// Add relevant columns to logical tile
logical_tile->AddColumns(tile_group, full_column_ids_);
Expand All @@ -374,6 +378,7 @@ bool IndexScanExecutor::ExecPrimaryIndexLookup() {
// Add the remaining tuples to the result vector
if ((current_tile_group_oid != INVALID_OID) && (!tuples.empty())) {
auto tile_group = storage_manager->GetTileGroup(current_tile_group_oid);
PELOTON_ASSERT(tile_group != nullptr);
std::unique_ptr<LogicalTile> logical_tile(LogicalTileFactory::GetTile());
// Add relevant columns to logical tile
logical_tile->AddColumns(tile_group, full_column_ids_);
Expand Down Expand Up @@ -464,6 +469,7 @@ bool IndexScanExecutor::ExecSecondaryIndexLookup() {
ItemPointer tuple_location = *tuple_location_ptr;
if (tuple_location.block != last_block) {
tile_group = storage_manager->GetTileGroup(tuple_location.block);
PELOTON_ASSERT(tile_group != nullptr);
tile_group_header = tile_group.get()->GetHeader();
}
#ifdef LOG_TRACE_ENABLED
Expand Down Expand Up @@ -565,6 +571,7 @@ bool IndexScanExecutor::ExecSecondaryIndexLookup() {
tuple_location =
*(tile_group_header->GetIndirection(tuple_location.offset));
tile_group = storage_manager->GetTileGroup(tuple_location.block);
PELOTON_ASSERT(tile_group != nullptr);
tile_group_header = tile_group.get()->GetHeader();
chain_length = 0;
continue;
Expand Down Expand Up @@ -594,6 +601,7 @@ bool IndexScanExecutor::ExecSecondaryIndexLookup() {

// search for next version.
tile_group = storage_manager->GetTileGroup(tuple_location.block);
PELOTON_ASSERT(tile_group != nullptr);
tile_group_header = tile_group.get()->GetHeader();
}
}
Expand Down Expand Up @@ -621,6 +629,7 @@ bool IndexScanExecutor::ExecSecondaryIndexLookup() {
// Since the tile_group_oids differ, fill in the current tile group
// into the result vector
auto tile_group = storage_manager->GetTileGroup(current_tile_group_oid);
PELOTON_ASSERT(tile_group != nullptr);
std::unique_ptr<LogicalTile> logical_tile(LogicalTileFactory::GetTile());
// Add relevant columns to logical tile
logical_tile->AddColumns(tile_group, full_column_ids_);
Expand All @@ -640,6 +649,7 @@ bool IndexScanExecutor::ExecSecondaryIndexLookup() {
// Add the remaining tuples (if any) to the result vector
if ((current_tile_group_oid != INVALID_OID) && (!tuples.empty())) {
auto tile_group = storage_manager->GetTileGroup(current_tile_group_oid);
PELOTON_ASSERT(tile_group != nullptr);
std::unique_ptr<LogicalTile> logical_tile(LogicalTileFactory::GetTile());
// Add relevant columns to logical tile
logical_tile->AddColumns(tile_group, full_column_ids_);
Expand Down Expand Up @@ -691,6 +701,7 @@ bool IndexScanExecutor::CheckKeyConditions(const ItemPointer &tuple_location) {

auto storage_manager = storage::StorageManager::GetInstance();
auto tile_group = storage_manager->GetTileGroup(tuple_location.block);
PELOTON_ASSERT(tile_group != nullptr);
ContainerTuple<storage::TileGroup> tuple(tile_group.get(),
tuple_location.offset);

Expand Down
6 changes: 6 additions & 0 deletions src/executor/seq_scan_executor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,12 @@ bool SeqScanExecutor::DExecute() {
while (current_tile_group_offset_ < table_tile_group_count_) {
auto tile_group =
target_table_->GetTileGroup(current_tile_group_offset_++);

if (tile_group == nullptr) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am confused that tile_group can be nullptr in this seq scan executor, but be asserted not be null in the previous index and hybrid scan executor ?

// tile group was freed so, continue to next tile group
continue;
}

auto tile_group_header = tile_group->GetHeader();

oid_t active_tuple_count = tile_group->GetNextTupleSlot();
Expand Down
Loading