Skip to content

Commit

Permalink
[#23943] YSQL: Fix Bitmap Scan GCC11 crash (follow-up)
Browse files Browse the repository at this point in the history
Summary:
89b69cf / D38080 attempted to fix the crash in GCC11, but after an initial successful run on Jenkins I made a small change and tested locally, omitting the `--gcc11` flag that was required to reproduce the crash. Since I had already seen a successful Jenkins run and believed the local results were still good, I didn't look closely at the Jenkins results and missed that it had started failing again on gcc11 fastdebug.

The previous hypothesis was that `delete[]`ing the memory caused the `Slice`'s data to point to a garbage value, which could potentially cause errors while erasing the slice from the unordered_set. This proved not to be true, as calling `Slice::Clear` after deleting the memory did not resolve the problem.

The problem is actually mentioned by the [[ https://en.cppreference.com/w/cpp/container/unordered_set | C++ standard ]]
> Container elements may not be modified (even by non const iterators) since modification could change an element's hash and corrupt the container.

Rearranging the instructions so that the modification of the Slice occurs after it is removed from the set solves the problem. This is also the same as the first draft of the previous attempt of a fix, but now the reason is well understood.
Jira: DB-12839

Test Plan:
Jenkins,

Manually:
```
Build:
./yb_build.sh fastdebug --gcc11
Restart cluster
Run
/*+ Set(yb_enable_bitmapscan true) Set(yb_enable_base_scans_cost_model true) BitmapScan(t) */
EXPLAIN (ANALYZE, SUMMARY OFF, COSTS OFF) SELECT * FROM test_and t WHERE a < 5 AND b < 5;

\watch .1
```

Reviewers: amartsinchyk

Reviewed By: amartsinchyk

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D38318
  • Loading branch information
timothy-e committed Sep 24, 2024
1 parent afc424d commit 76a5c97
Showing 1 changed file with 6 additions and 2 deletions.
8 changes: 6 additions & 2 deletions src/yb/yql/pggate/ybc_pggate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,6 @@ typedef std::unordered_set<Slice, Slice::Hash> UnorderedSliceSet;

static void FreeSlice(Slice slice) {
delete[] slice.data(), slice.size();
slice.Clear();
}

SliceSet YBCBitmapCreateSet() {
Expand Down Expand Up @@ -686,8 +685,13 @@ SliceSet YBCBitmapIntersectSet(SliceSet sa, SliceSet sb) {
auto iterb = b->begin();
for (auto itera = a->begin(); itera != a->end();) {
if ((iterb = b->find(*itera)) == b->end()) {
FreeSlice(*itera);
// We cannot modify the slice while it is in the set because
// std::unordered_set stores const Slices. Since the slice contains
// malloc'd memory, we grab a reference to it to delete the memory after
// removing it from the set.
auto data = itera->data();
itera = a->erase(itera);
delete[] data;
} else {
++itera;
}
Expand Down

0 comments on commit 76a5c97

Please sign in to comment.