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.

Sign up for GitHub

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

Jump to bottom

Improving on the parallelism idea #167

Closed
wants to merge 2 commits into from
Closed

Improving on the parallelism idea #167

wants to merge 2 commits into from

Conversation

spiderjako
Copy link
Contributor

@spiderjako spiderjako commented Dec 6, 2023 •
edited
Loading

Description

Done

  • Refactored the workLoop to work in the following manner: ⭕
  1. Add tasks to the queue, unless it's full (constant should be better defined, currently set at 100 by default)
  2. Take whatever is in the "buffer" array (maybe not a good name), sort it and take each value that has a block number in the current or the next interval. ⚠️ In the code under a TODO I've outlined a potential optimisation in regards to blocks without data that we need / empty blocks. Will be done by the end of the day (22.12)
  3. Store this non-descending (by block number) array in Kafka
  4. Save progress ⚠️ this would be solved with the TODO that I've outlined
  • Took optimisations from this PR (nextIntervalCalculator mainly) ⭕
  • Made optimisations to the order in which we fetch traces, blocks and receipts ⭕
  • Removed async/await from some methods and renamed them for better clarity ⭕

TODOs

  • PR is rebased and refactored using the master code (a lot of changes were made during the last 1-2 weeks) ❌
  • Final changes on how the progress is being stored and how to handle empty blocks (idea is in the TODOs in the code) ❌
  • Other blockchains are refactored in order to work with the new ❌
  • Add Tests, check existing tests, due name-changes ❌

spiderjako
spiderjako commented Dec 22, 2023
View reviewed changes
index.js
@@ -43,6 +50,10 @@ class Main {
this.worker = new worker.worker();
await this.worker.init(this.exporter, metrics);
await this.handleInitPosition();
if (BLOCKCHAIN === 'eth') {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

⚠️ Temporary: Currently testing with the Ethereum codebase. When done, would rework the rest of the code to work in the new way

@spiderjako spiderjako added the WIP label Dec 22, 2023
spiderjako
spiderjako commented Dec 22, 2023
View reviewed changes
blockchains/eth/eth_worker.js
this.lastExportedBlock = result.toBlock;

return events;
this.lastExportedBlock = Math.max(intervals[intervals.length - 1].toBlock, this.lastExportTime);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This doesn't make sense ATM, should be removed/refactored

@spiderjako spiderjako mentioned this pull request Jan 7, 2024
Open
@spiderjako
Copy link
Contributor Author

Closing in favour of this PR

@spiderjako spiderjako closed this Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@YordanPavlov YordanPavlov Awaiting requested review from YordanPavlov

@WonderBeat WonderBeat Awaiting requested review from WonderBeat

Assignees
No one assigned
Labels
WIP
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@spiderjako