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

kad: Provide partial results to speedup GetRecord queries #315

Merged
merged 12 commits into from
Jan 14, 2025

Conversation

lexnv
Copy link
Collaborator

@lexnv lexnv commented Jan 8, 2025

This PR provides the partial results of the GetRecord kademlia query.

  • A new GetRecordPartialResult event is introduced for kademlia
  • GetRecordSuccess is modified to include only the query ID
  • Kademlia GetRecord implementation no longer stores network records internally and forwards valid (unexpired) ones back to the user

The change is needed to speedup authority discovery for substrate based chains.

More context can be found at: paritytech/polkadot-sdk#7077 (comment)

Next Steps

  • Adjust testing to API breaking change
  • Investigate CPU impact (as suggested by @dmitry-markin this should be unnoticeable 🙏 )

lexnv added 5 commits January 8, 2025 10:44
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
@lexnv lexnv self-assigned this Jan 8, 2025
Copy link
Collaborator

@dmitry-markin dmitry-markin left a comment

Choose a reason for hiding this comment

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

Nice!

Comment on lines 489 to 501
// Send partial response to the user.
if let Some(QueryAction::GetRecordPartialResult { query_id, record }) =
self.engine.register_response(
query_id,
peer,
KademliaMessage::GetRecord { key, record, peers },
)
{
let _ = self
.event_tx
.send(KademliaEvent::GetRecordPartialResult { query_id, record })
.await;
}
Copy link
Collaborator

@dmitry-markin dmitry-markin Jan 8, 2025

Choose a reason for hiding this comment

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

nit: to keep the structure of event handling, I would let the engine pass QueryAction::GetRecordPartialResult to on_query_action() and published the outbound event there. The latency won't be noticeable, but the code structure will be more streamlined.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, that makes sense! 🙏 Let me know if the latest changes are oki with you, planning to do a release later today

lexnv added 7 commits January 8, 2025 15:12
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
@lexnv lexnv requested a review from dmitry-markin January 14, 2025 09:24
@lexnv lexnv merged commit c6a0746 into master Jan 14, 2025
8 checks passed
@lexnv lexnv deleted the lexnv/kad-found-records branch January 14, 2025 13:37
lexnv added a commit that referenced this pull request Jan 15, 2025
## [0.9.0] - 2025-01-14

This release provides partial results to speed up `GetRecord` queries in
the Kademlia protocol.

### Changed

- kad: Provide partial results to speedup `GetRecord` queries
([#315](#315))


cc @paritytech/networking

---------

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
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.

2 participants