Implement DeleteIndexes

[gammazero]

No description provided.

[codecov-commenter]

Codecov Report

Attention: 49 lines in your changes are missing coverage. Please review.

Comparison is base (cea5ef7) 57.44% compared to head (11a4d1a) 56.49%.

Files	Patch %	Lines
pebble/pebble.go	44.28%	34 Missing and 5 partials ⚠️
server/server.go	54.54%	8 Missing and 2 partials ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #94      +/-   ##
==========================================
- Coverage   57.44%   56.49%   -0.95%     
==========================================
  Files          12       12              
  Lines         947     1039      +92     
==========================================
+ Hits          544      587      +43     
- Misses        359      401      +42     
- Partials       44       51       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[masih]

Thank you for picking this up 👍 Left some suggestions re alternatives.

it would be great to also have benchmarks for deletions (not necessarily in this PR).

[masih]

I have a suspicion that this would be slow at scale. But it is certainly a good first pass in implementing deletion 👍

Before doing it any other way, I recommend adding benchmarks. We can then iterate on alternative approaches.

---

On alternative approaches:

I have thought of 2 other ways of doing this, which can be mixed and matched for a high performance opportunistic deletion mechanism:

1. Accumulate indexes marked for deletion, sort them then delete in much larger balks using delete range.
2. Update the merger implementation to intelligently recognise entires pending deletion and use merging function to exclude them from the merge, which will effectively result in deletion. This is the same approach I originally implemented in the non-encrypted pebble for opportunistic deletion.

[gammazero]

This is at least a batched delete (batch.Delete and not s.db.Delete) so all the deletions are collected in a batch and done in one shot and it should be faster than a large number of individual calls to s.db.Delete.

Since this is done by GC, and is not in the critical path of indexing or lookup, it is OK for it to be slow. I think, for us, it is more important to make sure that no deletion work is done in the indexing and lookup path, even if that makes deletion less optimal. GC can crawl along in the background at a very slow pace, as long as it does not interfere with indexing or lookup.

The suggested approaches are good in general, but I think that 2 requires some work in the ingestion path during merge processing. Number 1 may be more efficient, but if it only improves GC then the efficiency does not matter at the cost of storing temporary data (which does have a cost - compaction cost).

[masih]

I suspect that the compaction cost would be lower with range deletion. For approach No. 2 i think all the changes would remain in this repo if by ingestion path changes we mean changes in storetheindex?

Thank you for implementing the batch deletion. That should help us gather some data to see if it needs optimisation at all 👍

[gammazero]

By ingestion path, I mean during the process of ingestion. I want writing new index records to remain free from anything other than writing new records.