Skip to content

Conversation

@guillaumemichel
Copy link
Collaborator

@guillaumemichel guillaumemichel commented Sep 4, 2025

Supersedes #1140

Fixes #1112

Part of #1095

KeyStore Revamp

  • Change indexing strategy
  • Size() function that will be used for Stats
  • Removing Garbage Collection logic
  • Batching Put and Delete operations

@guillaumemichel guillaumemichel force-pushed the provider-keystore-revamp-2 branch from a7c83de to 8f262c9 Compare September 4, 2025 15:42
Copy link
Contributor

@gammazero gammazero left a comment

Choose a reason for hiding this comment

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

The code looks good, but I am concerned about performance given that each bit of the key needs a path separator.

Data stores such a pebble do allow for iterating items by specifying bitwise key prefix bounds (see here). It may be worth providing a way to access this functionality through our datastore/query wrappers so that datastore queries can operate on a bitwise key prefix instead of only a path prefix, for datastores that support it.

Seems like it would doable with go-ds-pebble, here.

We can release the provider keystore as it currently is, or even with this PR if performance does not suffer, and then upgrade to an implementation that does bitwise key prefix iteration later.

@guillaumemichel
Copy link
Collaborator Author

guillaumemichel commented Sep 5, 2025

@gammazero I share your concerns. See #1143 for optimization attempts.

Forcing to use pebble might be a good thing (either supplied as option, or a new pebble db is created for the provider)-

Using pebble would help reduce the key size, but it would give us prefix matching on bytes (instead of / separated strings), but ultimately we need to match on bits.

If we need to match on a bitstring on length 7, using pebble, it means we would match absolutely all the keys from the store (because they all match up to the byte that isn't fully covered by the bitstring), and we then need to filter the query results. I am not sure how much of an overhead it represents.

In the current solution, we basically match on 4 bits, which is slightly better in that regard, but we have the / separator overhead.


One other option would be to use separators only for the first (16-32) bits, and then an efficient encoding for the last (224-240) bits. To allow for optimal bit-wise prefix match we could have prefixes using:

  1. Base prefix: /base/prefix
  2. Binary for first 8 or 12 bits: /1/0/0/0/1/...
  3. Hex for the next 8 to 24 bits: /e/9/1/0/...
  4. Base64 with escaping the / or more efficient encoding for the remaining (224-240) bits: /j8K2mPxQ7vL9R3nE4sA6BcF1wYzN5oP8qX2rK9sT6hU=

In this example we get at most 18 /s in the 256-bit key encoding, in comparison with 64 currently.

This would allow for efficient bit-wise matching for short prefixes, while remaining compatible with go-datastore interface, and limiting the number of / separators.

@guillaumemichel guillaumemichel merged commit 55678db into provider Sep 8, 2025
9 checks passed
@guillaumemichel guillaumemichel deleted the provider-keystore-revamp-2 branch September 8, 2025 17:25
guillaumemichel added a commit to guillaumemichel/go-libp2p-kad-dht that referenced this pull request Sep 17, 2025
* keystore: revamp

* fix: KeyStore.Close dependencies

* keystore: implement KeyStore4
guillaumemichel added a commit to guillaumemichel/go-libp2p-kad-dht that referenced this pull request Sep 17, 2025
* keystore: revamp

* fix: KeyStore.Close dependencies

* keystore: implement KeyStore4
guillaumemichel added a commit that referenced this pull request Sep 17, 2025
* keystore: revamp

* fix: KeyStore.Close dependencies

* keystore: implement KeyStore4
guillaumemichel added a commit that referenced this pull request Sep 17, 2025
* keystore: revamp

* fix: KeyStore.Close dependencies

* keystore: implement KeyStore4
guillaumemichel added a commit that referenced this pull request Sep 18, 2025
* keystore: revamp

* fix: KeyStore.Close dependencies

* keystore: implement KeyStore4
guillaumemichel added a commit that referenced this pull request Sep 18, 2025
* keystore: revamp

* fix: KeyStore.Close dependencies

* keystore: implement KeyStore4
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