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

ethdb: add DeleteRange feature #30668

Merged
merged 5 commits into from
Oct 25, 2024
Merged

Conversation

zsfelfoldi
Copy link
Contributor

This PR adds DeleteRange to ethdb.KeyValueWriter. While range deletion using an iterator can be really slow, DeleteRange is natively supported by pebble and apparently runs in O(1) time (typically 20-30ms in my tests for removing hundreds of millions of keys and gigabytes of data). For leveldb and memorydb an iterator based fallback is implemented. Note that since the iterator method can be slow and a database function should not unexpectedly block for a very long time, the number of deleted keys is limited at 10000 which should ensure that it does not block for more than a second. ErrTooManyKeys is returned if the range has only been partially deleted. In this case the caller can repeat the call until it finally succeeds.

Copy link
Member

@rjl493456442 rjl493456442 left a comment

Choose a reason for hiding this comment

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

nitpicks, otherwise lgtm

@zsfelfoldi
Copy link
Contributor Author

@rjl493456442 nitpicks addressed :)

Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

Looks good to me!

I'm not 100% I agree with throwing TooManyKeys at exactly 1000 keys, but I guess in practice it's kind of moot: it's not going to be used on leveldb anyway, at some point we'll drop leveldb and solely do pebble, so I think this is good enough

@zsfelfoldi zsfelfoldi merged commit 80bdab7 into ethereum:master Oct 25, 2024
3 checks passed
@zsfelfoldi
Copy link
Contributor Author

Looks good to me!

I'm not 100% I agree with throwing TooManyKeys at exactly 1000 keys, but I guess in practice it's kind of moot: it's not going to be used on leveldb anyway, at some point we'll drop leveldb and solely do pebble, so I think this is good enough

I also don't love it but i think there is no perfect solution here. This is simple and predictable and avoids unacceptable scenarios.

@zsfelfoldi zsfelfoldi added this to the 1.14.12 milestone Oct 25, 2024
holiman pushed a commit that referenced this pull request Nov 19, 2024
This PR adds `DeleteRange` to `ethdb.KeyValueWriter`. While range
deletion using an iterator can be really slow, `DeleteRange` is natively
supported by pebble and apparently runs in O(1) time (typically 20-30ms
in my tests for removing hundreds of millions of keys and gigabytes of
data). For leveldb and memorydb an iterator based fallback is
implemented. Note that since the iterator method can be slow and a
database function should not unexpectedly block for a very long time,
the number of deleted keys is limited at 10000 which should ensure that
it does not block for more than a second. ErrTooManyKeys is returned if
the range has only been partially deleted. In this case the caller can
repeat the call until it finally succeeds.
zfy0701 pushed a commit to sentioxyz/go-ethereum that referenced this pull request Dec 3, 2024
This PR adds `DeleteRange` to `ethdb.KeyValueWriter`. While range
deletion using an iterator can be really slow, `DeleteRange` is natively
supported by pebble and apparently runs in O(1) time (typically 20-30ms
in my tests for removing hundreds of millions of keys and gigabytes of
data). For leveldb and memorydb an iterator based fallback is
implemented. Note that since the iterator method can be slow and a
database function should not unexpectedly block for a very long time,
the number of deleted keys is limited at 10000 which should ensure that
it does not block for more than a second. ErrTooManyKeys is returned if
the range has only been partially deleted. In this case the caller can
repeat the call until it finally succeeds.
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

Successfully merging this pull request may close these issues.

3 participants