Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CompactRange (c++) causes segfault #596

Closed
stephan-hof opened this issue Apr 28, 2015 · 4 comments
Closed

CompactRange (c++) causes segfault #596

stephan-hof opened this issue Apr 28, 2015 · 4 comments

Comments

@stephan-hof
Copy link

Hi guys,

I need a bit of support. In my application code there is this line

this->db->CompactRange(this->cf_handle, NULL, NULL, true);

Unfortunately the program aborts with a segfault. I already attached gdb to it having the following back-trace

#0  rocksdb::SomeFileOverlapsRange (icmp=..., disjoint_sorted_files=<optimized out>, file_level=..., smallest_user_key=0x0, largest_user_key=0x0) at db/version_set.cc:398
#1  0x00007ffff5e80d74 in rocksdb::DBImpl::CompactRange (this=0x1088750, column_family=<optimized out>, begin=0x0, end=0x0, reduce_level=true, target_level=-1, target_path_id=0)
    at db/db_impl.cc:1158
#2  0x00007ffff5e3cab5 in DataStore::compact (this=0xde2598) at cpp/data_store.cpp:151

I investigated the rocksdb code a bit and the following two parts puzzle me how they interact with 'level_files_brief_'

/* Part one. */
Status DBImpl::CompactRange(C.....
    for (int level = 1; level < cfd->NumberLevels(); level++) {
      if (base->storage_info()->OverlapInLevel(level, begin, end)) {

bool VersionStorageInfo::OverlapInLevel(int level,
                                        const Slice* smallest_user_key,
                                        const Slice* largest_user_key) {
  return SomeFileOverlapsRange(*internal_comparator_, (level > 0),
                               level_files_brief_[level], smallest_user_key,
                               largest_user_key);
}

In my case NumberLevels() is 10.

/* Part two. */
void VersionStorageInfo::GenerateLevelFilesBrief() {
  level_files_brief_.resize(num_non_empty_levels_);
  for (int level = 0; level < num_non_empty_levels_; level++) {

In my case num_non_empty_levels_ is 2.

From my point of view OverlapInLevel is called with level=0,1,2,3,4,5...9 however level_files_brief_ has only size 2. So there is an 'out of range' access. I'm using 3.9.fb, but the code in question is also in master.

If you need more input from my side feel free to ask. It is easy to reproduce and the environment is under full control.

@igorcanadi
Copy link
Collaborator

Thanks for a great bug report!

Wow I'm not sure how we never caught this before. Fix here: https://reviews.facebook.net/D37779

@igorcanadi
Copy link
Collaborator

(BTW to test it out, you can do arc patch D37779 on the repository. You'll need to install arcanist -- https://secure.phabricator.com/book/phabricator/article/arcanist/)

@stephan-hof
Copy link
Author

Hi the patch works in my scenario. Thanks for the quick response.

@igorcanadi
Copy link
Collaborator

Glad to help @stephan-hof !

igorcanadi added a commit that referenced this issue Apr 29, 2015
Summary: For very detailed explanation of what's happening read this: #596

Test Plan: make check + new unit test

Reviewers: yhchiang, anthony, rven

Reviewed By: rven

Subscribers: adamretter, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D37779
igorcanadi added a commit that referenced this issue Apr 30, 2015
Summary: For very detailed explanation of what's happening read this: #596

Test Plan: make check + new unit test

Reviewers: yhchiang, anthony, rven

Reviewed By: rven

Subscribers: adamretter, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D37779

Conflicts:
	db/db_test.cc
facebook-github-bot pushed a commit that referenced this issue Jan 12, 2018
Summary:
This test often causes out-of-space error when run on travis. We don't want such stress tests in our unit test suite.

The bug in #596, which this test intends to expose, can be repro'd as long as the bottommost level(s) are empty when CompactRange is called. I rewrote the test to cover this simple case without writing a lot of data.
Closes #3362

Differential Revision: D6710417

Pulled By: ajkr

fbshipit-source-id: 9a1ec85e738c813ac2fee29f1d5302065ecb54c5
amytai pushed a commit to amytai/rocksdb that referenced this issue Mar 9, 2018
Summary:
This test often causes out-of-space error when run on travis. We don't want such stress tests in our unit test suite.

The bug in facebook#596, which this test intends to expose, can be repro'd as long as the bottommost level(s) are empty when CompactRange is called. I rewrote the test to cover this simple case without writing a lot of data.
Closes facebook#3362

Differential Revision: D6710417

Pulled By: ajkr

fbshipit-source-id: 9a1ec85e738c813ac2fee29f1d5302065ecb54c5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants