Skip to content

Conversation

@dlg99
Copy link
Contributor

@dlg99 dlg99 commented Feb 11, 2022

Descriptions of the changes in this PR:

Motivation

RocksDB segfaulted during CompactionTest

Changes

RocksDB can segfault if one tries to use it after close.
Shutdown/compaction sequence can lead to such situation. The fix prevents segfault.

CompactionTests were updated at some point to use metadata cache and non-cached case is not tested.
I added the test suites for this case.

Master Issue: #3040

Copy link
Contributor

@nicoloboschi nicoloboschi left a comment

Choose a reason for hiding this comment

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

good catch, I left a question

gcStats.getMinorCompactionCounter().inc();
minorCompacting.set(false);
}
if (force && forceGarbageCollection.compareAndSet(true, false)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why did you move this statement inside the above else if ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed. I meant to move under finally, but under different finally.

@dlg99 dlg99 added this to the 4.15.0 milestone Feb 15, 2022
@dlg99 dlg99 merged commit 9b1b188 into apache:master Feb 15, 2022
StevenLuMT pushed a commit to StevenLuMT/bookkeeper that referenced this pull request Feb 16, 2022
Descriptions of the changes in this PR:

### Motivation

RocksDB segfaulted during CompactionTest

### Changes

RocksDB can segfault if one tries to use it after close.
[Shutdown/compaction sequence](apache#3040 (comment)) can lead to such situation. The fix prevents segfault.

CompactionTests were updated at some point to use metadata cache and non-cached case is not tested. 
I added the test suites for this case.

Master Issue: apache#3040 



Reviewers: Yong Zhang <zhangyong1025.zy@gmail.com>, Nicolò Boschi <boschi1997@gmail.com>

This closes apache#3043 from dlg99/fix/issue3040, closes apache#3040
Ghatage pushed a commit to sijie/bookkeeper that referenced this pull request Jul 12, 2024
Descriptions of the changes in this PR:

### Motivation

RocksDB segfaulted during CompactionTest

### Changes

RocksDB can segfault if one tries to use it after close.
[Shutdown/compaction sequence](apache#3040 (comment)) can lead to such situation. The fix prevents segfault.

CompactionTests were updated at some point to use metadata cache and non-cached case is not tested. 
I added the test suites for this case.

Master Issue: apache#3040 



Reviewers: Yong Zhang <zhangyong1025.zy@gmail.com>, Nicolò Boschi <boschi1997@gmail.com>

This closes apache#3043 from dlg99/fix/issue3040, closes apache#3040
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants