Skip to content

Conversation

NumberFour8
Copy link
Contributor

@NumberFour8 NumberFour8 commented Mar 22, 2024

This PR introduces the following changes:

  • get_indexer_data and get_safe_info DB calls are now using cache. The Indexer data especially is query very frequently across the entire code base, and changes very sporadically. Therefore, proper caching has been introduced along with the invalidation when the underlying data changes in the DB.

  • resolve_chain_key and resolve_packet_key also use cache, so packet processing pipeline can benefit from it. These calls are very often called directly from the packet processing pipeline.

  • All caches that are used by the DB and TicketManager have been moved into a separate object. This object is then referenced via Arc in the DB and TicketManager to allow fast cloning.

  • set_last_indexed_block was separated into a standalone method, to avoid invalidating Indexer Data cache too often.

  • Integration tests were enhanced

  • Several unused features and dependencies were removed

Closes #6045

@NumberFour8 NumberFour8 self-assigned this Mar 22, 2024
@github-actions github-actions bot added the dependencies Pull requests that update a dependency file label Mar 22, 2024
@NumberFour8 NumberFour8 requested a review from a team March 22, 2024 17:44
@NumberFour8 NumberFour8 marked this pull request as ready for review March 22, 2024 18:01
Copy link
Contributor

@Teebor-Choka Teebor-Choka left a comment

Choose a reason for hiding this comment

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

I'm not sure how much of that would be effectively cached by the SQL database though.

@@ -18,24 +18,41 @@ pub trait HoprDbResolverOperations {
#[async_trait]
impl HoprDbResolverOperations for HoprDb {
async fn resolve_packet_key(&self, onchain_key: &Address) -> Result<Option<OffchainPublicKey>> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Theoretically this API could disappear and the OffchainKeyOrAddress could take a Arc reference and resolve dynamically into either Address or OffchainKey, this creating a sort of "UniversalAddress".

@@ -1397,6 +1392,7 @@ impl HoprDbTicketOperations for HoprDb {
))
})?;

// TODO: cache this DB call too, or use the channel graph
Copy link
Contributor

Choose a reason for hiding this comment

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

The question is whether the SQLite cache is not sufficient, not only here, but in multiple tables and values that rarely change.

@NumberFour8 NumberFour8 merged commit 303e503 into master Mar 22, 2024
@NumberFour8 NumberFour8 deleted the lukas/cache-indexer-data branch March 22, 2024 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complexity:normal how hard is the review component:db component:tests dependencies Pull requests that update a dependency file effort:medium time needed to complete review improvement
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Make PeerId <-> Address resolution use a cache
2 participants