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

Improve lifetime management in RangeDelAggregator #4807

Open
abhimadan opened this issue Dec 20, 2018 · 1 comment
Open

Improve lifetime management in RangeDelAggregator #4807

abhimadan opened this issue Dec 20, 2018 · 1 comment
Labels
up-for-grabs Up for grabs

Comments

@abhimadan
Copy link
Contributor

Currently, any range tombstone iterator passed to RangeDelAggregator is retained for the aggregator's entire lifetime. For long range scans, this can have a significant memory cost. Ideally, we would free iterators when the top-level DBIter moves past its corresponding file's range (L1+ only), and re-create them if DBIter moves back to the file. This would manifest as a RemoveTombstones method that is called by LevelIterator.

The problem with this approach is that a LevelIterator advances files after the last point key in the file is read, even if there is a gap between this key and the start of the next file that is covered by a range tombstone. We do not want to preemptively destroy the iterator containing this range tombstone, as it might cover keys in lower levels.

One solution to this problem is to insert sentinel keys between files in a LevelIterator if a file boundary has type kTypeRangeDeletion. These keys can be skipped by DBIter, but since this key will be stored in the underlying heap of iterators, it will keep the tombstones in the file alive for as long as they need to be alive.

@abhimadan abhimadan added the up-for-grabs Up for grabs label Dec 20, 2018
@ljluestc
Copy link

struct SentinelKey {
};

bool IsSentinelKey(const Slice& key) {
}

void LevelIterator::InsertSentinelKeys() {
  for (auto& file : files_) {
    if (file.type == kTypeRangeDeletion) {
      SentinelKey sentinel_key;

      keys_.push_back(sentinel_key);
      values_.push_back(/* corresponding value */);
    }
  }
}

void DBIter::Next() {
  do {
    ++idx_;
    if (IsSentinelKey(keys_[idx_])) {
      continue;
    }
  } while (idx_ < keys_.size());
}

void RecreateRangeTombstoneIterators(/* necessary parameters */) {
}

void LevelIterator::Next() {
  ++idx_;
  if (IsSentinelKey(keys_[idx_])) {
    RecreateRangeTombstoneIterators(/* necessary parameters */);
    ++idx_;
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
up-for-grabs Up for grabs
Projects
None yet
Development

No branches or pull requests

2 participants