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

[BESU-169] cache logs bloom filters automatically. #367

Merged
merged 28 commits into from
Feb 13, 2020

Conversation

AbdelStark
Copy link
Contributor

@AbdelStark AbdelStark commented Feb 5, 2020

PR description

Log index is generated either by starting Besu using the generate-log-bloom-cache CLI subcommand or by calling the admin_generatelogbloomcache RPC method.
However, it seems necessary to make this call or run this subcommand every once a while to regenerate log index cache.
A workaround is to have a cron task sending an RPC request to admin_generatelogbloomcache, but then the return result have to be stored to be used as parameters for the next call.

It then would be great to have Besu able to update the index cache by itself on a configurable schedule (every once x seconds, or every once x blocks, or automatically by letting Besu decide the best moment to update) and with an automatic management of the range to index (without the need of the two params the admin_generatelogbloomcache requires).

Fixed Issue(s)

https://jira.hyperledger.org/browse/BESU-169

Documentation changes required

  • Added --auto-logs-bloom-indexing-enabled CLI option: Enable Automatic logs bloom indexing

TODO

  • Implement a hook to cache missing logs bloom at startup DONE
  • Implement a hook to cache missing logs bloom on new block propagated. iterate over all previous segments to see if they are missing, trigger indexing if some are missing. DONE

Signed-off-by: Abdelhamid Bakhta [email protected]

Signed-off-by: Abdelhamid Bakhta <[email protected]>
@AbdelStark AbdelStark added the enhancement New feature or request label Feb 5, 2020
@AbdelStark AbdelStark self-assigned this Feb 5, 2020
Signed-off-by: Abdelhamid Bakhta <[email protected]>
Signed-off-by: Abdelhamid Bakhta <[email protected]>
Signed-off-by: Abdelhamid Bakhta <[email protected]>
@AbdelStark AbdelStark marked this pull request as ready for review February 5, 2020 17:37
@AbdelStark AbdelStark added the doc-change-required Indicates an issue or PR that requires doc to be updated label Feb 10, 2020
@AbdelStark AbdelStark changed the title [WIP][BESU-169] Index logs automatically. [BESU-169] Index logs automatically. Feb 10, 2020
@RatanRSur
Copy link
Contributor

Somewhere around here!

while (currentOldChainWithReceipts.getNumber() > currentNewChainWithReceipts.getNumber()) {

Signed-off-by: Abdelhamid Bakhta <[email protected]>
Signed-off-by: Abdelhamid Bakhta <[email protected]>
Signed-off-by: Abdelhamid Bakhta <[email protected]>
@AbdelStark AbdelStark force-pushed the besu-169/auto-log-indexing branch from 3a6861a to 8a3c988 Compare February 12, 2020 16:53
shemnon
shemnon previously approved these changes Feb 12, 2020
@shemnon shemnon changed the title [BESU-169] Index logs automatically. [BESU-169] cache logs bloom filters automatically. Feb 12, 2020
@shemnon shemnon dismissed their stale review February 12, 2020 22:14

need some verbiage changes

Copy link
Contributor

@shemnon shemnon left a comment

Choose a reason for hiding this comment

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

So it should be 'caching' not 'indexing' everywhere. And 'auto log bloom cache' to align from the existing CLI verbiage. Log is singular in this case.

@@ -257,6 +257,9 @@ public void startNode(final BesuNode node) {
params.add("--key-value-storage");
params.add("rocksdb");

params.add("--auto-logs-bloom-indexing-enabled");
Copy link
Contributor

@shemnon shemnon Feb 12, 2020

Choose a reason for hiding this comment

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

Suggested change
params.add("--auto-logs-bloom-indexing-enabled");
params.add("--auto-log-bloom-caching-enabled");

@@ -195,6 +195,7 @@ public void startNode(final BesuNode node) {
.map(EnodeURL::fromString)
.collect(Collectors.toList()))
.besuPluginContext(new BesuPluginContextImpl())
.autoLogsBloomIndexing(false)
Copy link
Contributor

@shemnon shemnon Feb 12, 2020

Choose a reason for hiding this comment

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

Suggested change
.autoLogsBloomIndexing(false)
.autoLogBloomCaching(false)

public void cacheLogsBloomForBlockHeader(final BlockHeader blockHeader) {
try {
final long blockNumber = blockHeader.getNumber();
LOG.info("Caching logs bloom for block {}.", "0x" + Long.toHexString(blockNumber));
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be debug - it happens way too often.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -60,6 +67,10 @@ public TransactionLogsIndexer(
this.scheduler = scheduler;
}

public IndexingStatus indexAll() {
return generateLogBloomCache(0, Long.MAX_VALUE);
Copy link
Contributor

Choose a reason for hiding this comment

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

This can result in indexing running twice... perhaps ensurePreviousSegmentsArePresent(blockchain.getChainHeadBlockNumber());?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, good suggestion. Done.

long currentSegment = (blockNumber / BLOCKS_PER_BLOOM_CACHE) - 1;
while (currentSegment > 0) {
try {
if (!isCachePresentForSegment(currentSegment)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should track what segments we have made in a list or tree map. Check the map before going to disk.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

}

private void ensurePreviousSegmentsArePresent(final long blockNumber) {
scheduler.scheduleFutureTask(
Copy link
Contributor

Choose a reason for hiding this comment

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

We should check indexing status before submitting the work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

@shemnon shemnon left a comment

Choose a reason for hiding this comment

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

Renaming from indexing to caching should also extend to the mistakes of the previous author (me!). TransactionLogsIndexer -> TransactionLogsCacher and IndexingStatus -> CachingStatus

Copy link
Contributor

@shemnon shemnon left a comment

Choose a reason for hiding this comment

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

On third thought, save the renaming for a follow on PR and focus on the other comments I left for implementation details.

Signed-off-by: Abdelhamid Bakhta <[email protected]>
Signed-off-by: Abdelhamid Bakhta <[email protected]>
Signed-off-by: Abdelhamid Bakhta <[email protected]>
@AbdelStark AbdelStark merged commit 6677362 into hyperledger:master Feb 13, 2020
@AbdelStark AbdelStark deleted the besu-169/auto-log-indexing branch February 17, 2020 09:31
shemnon pushed a commit to shemnon/besu that referenced this pull request Feb 19, 2020
* First iteration. Draft PR.

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* fix SPDX header

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* Use block broadcaster to index log bloom.

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* Remove useless toString method

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* spotless apply

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* cacheLogsBloomForBlockHeader

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* spotless apply

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* ensurePreviousSegmentsArePresent

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* Added CLI flag to enable / disable automatic logs bloom indexing.

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* Create cache directory and cache file if not exist.

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* Fix acceptance test

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* Write cache for block only if block is new canonical head.

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* Handling of chain reorg.

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* fix

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* sportless apply

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* Address PR comments.

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* Remove unused constant.

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* spotless apply

Signed-off-by: Abdelhamid Bakhta <[email protected]>
(cherry picked from commit 6677362)
Signed-off-by: Danno Ferrin <[email protected]>
GregTheGreek pushed a commit to ChainSafe/besu that referenced this pull request Feb 27, 2020
* Plugin error stack traces (hyperledger#369)

Because of how the Log4J2 api works exception stack traces were not
being printed.  Update to use the explicit "throwable" overloaded
methods.

Signed-off-by: Danno Ferrin <[email protected]>

* VM Trace fixes (hyperledger#372)

* correct refund addresses
* correct returned memory from static precompiled calls.
* update integration test
* precompiles sometimes get plain old CALLs

Signed-off-by: Danno Ferrin <[email protected]>

* Validate private transaction before sending to enclave (hyperledger#356)

Signed-off-by: Jason Frame <[email protected]>

* Trace API fixes (hyperledger#377)

- Correct Reporting of reverts in nested call
- correct reporting and handling of value transfer in nested calls
- correct handling of precompiles via DELEGATECALL & CALLCODE
- Addition of precompiled contract gas costs
- Re-work handling of storage writes
- Initial handling of gas refunds
- fix bug in DELEGATECALL tests, we don't need gas in the stack
  * this has a cascading effect on balances in diff tests
- rework depth detection in flat trace
- two new tests blocks

Signed-off-by: Danno Ferrin <[email protected]>

* [BOUNTY-2] Add NAT Docker Support  (hyperledger#368)

* add docker detection

Signed-off-by: Karim TAAM <[email protected]>

* add port mapping detection

Signed-off-by: Karim TAAM <[email protected]>

* add tests and refactor ip detection

Signed-off-by: Karim TAAM <[email protected]>

* clean RunnerBuilder

Signed-off-by: Karim TAAM <[email protected]>

* clean useless modification

Signed-off-by: Karim TAAM <[email protected]>

* spotless

Signed-off-by: Karim TAAM <[email protected]>

* resolve tests issues

Signed-off-by: Karim TAAM <[email protected]>

* streamline auto detection

Signed-off-by: Ratan Rai Sur <[email protected]>

Co-authored-by: Abdelhamid Bakhta <[email protected]>
Co-authored-by: Ratan Rai Sur <[email protected]>

* [PIE-1798] Priv RPC acceptance tests with stub enclave. (hyperledger#330)

* [PIE-1798] Added some Privacy RPC ATs with a stub enclave.

Signed-off-by: Mark Terry <[email protected]>

* More specific task metrics names (hyperledger#389)

A prior refactoring had accidentally removed the specific task names
from the metrics labels.

Signed-off-by: Danno Ferrin <[email protected]>

* More trace fixes (hyperledger#386)

* pop flat trace context when handling halts
* Better detection of precompiled and non-executed contracts
* correct from address when calling in init code
* fix some exotic nesting cases
* correct from field for init code calls at depth >1
* correct cost on a non-call
* changelog and notes

Signed-off-by: Danno Ferrin <[email protected]>

* adding the plugin-api javadoc jar at the root level (hyperledger#378)

Signed-off-by: Joshua Fernandes <[email protected]>

* BESU-56: remove erroneous links to errorprone checks (hyperledger#385)

Signed-off-by: Antoine Toulme <[email protected]>

* Changelog entry for multi-tenancy feature (hyperledger#394)

Signed-off-by: Edward Evans <[email protected]>
Signed-off-by: Jason Frame <[email protected]>

* 1.5 RC Changelog (hyperledger#395)

* 1.5 RC changelog additions

Signed-off-by: Sally MacFarlane <[email protected]>

* Fixed link (hyperledger#393)

Signed-off-by: Madeline <[email protected]>

Co-authored-by: Edward <[email protected]>

* docker changelog (hyperledger#391)

* docker changelog

Signed-off-by: Ratan Rai Sur <[email protected]>

* address comments

Signed-off-by: Ratan Rai Sur <[email protected]>

Co-authored-by: MadelineMurray <[email protected]>

* Added known bug to changelog  (hyperledger#388)

* Added known bug

Signed-off-by: Madeline <[email protected]>

* Added another known bug

Signed-off-by: Madeline <[email protected]>

* updating version to 1.4.0-rc1 (hyperledger#397)

Signed-off-by: Joshua Fernandes <[email protected]>

* updating verion to 1.4.1-snapshot (hyperledger#398)

Signed-off-by: Joshua Fernandes <[email protected]>

* [BESU-169] cache logs bloom filters automatically. (hyperledger#367)

* First iteration. Draft PR.

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* fix SPDX header

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* Use block broadcaster to index log bloom.

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* Remove useless toString method

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* spotless apply

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* cacheLogsBloomForBlockHeader

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* spotless apply

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* ensurePreviousSegmentsArePresent

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* Added CLI flag to enable / disable automatic logs bloom indexing.

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* Create cache directory and cache file if not exist.

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* Fix acceptance test

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* Write cache for block only if block is new canonical head.

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* Handling of chain reorg.

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* fix

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* sportless apply

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* Address PR comments.

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* Remove unused constant.

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* spotless apply

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* [BESU-25] Use Devp2p Ping packets at v5 (hyperledger#392)

Broadcast that we support snappy compression

Signed-off-by: Danno Ferrin <[email protected]>

* Rename logs bloom indexer to log bloom cache to match CLI flag. (hyperledger#401)

Changes class names, variables, and CLI flags as needed.

Signed-off-by: Danno Ferrin <[email protected]>

* [PIE-1798] Fail cases for multitenancy ATs (hyperledger#400)

Signed-off-by: Mark Terry <[email protected]>

* Private state update metadata and migration (hyperledger#404)

(backport from release-1.4)
Private state update metadata and migration

Signed-off-by: Lucas Saldanha <[email protected]>

* [PIE-2303] Automatic log bloom caching - Remove usage of pending file. (hyperledger#407)

* Don't use pending file.

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* Don't use pending file.

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* Create a custom error when plugin is not found. (hyperledger#409)

Signed-off-by: Abdelhamid Bakhta <[email protected]>

* Rename method (hyperledger#412)

* rename the method isPersistingState to isPersistingPrivateState because that is what it is used for

Signed-off-by: Stefan Pingel <[email protected]>

* rename the method isPersistingState to isPersistingPrivateState because that is what it is used for

Signed-off-by: Stefan Pingel <[email protected]>

* rename the method isPersistingState to isPersistingPrivateState because that is what it is used for

Signed-off-by: Stefan Pingel <[email protected]>

* [BOUNTY-4] Add NAT Kubernetes Support (hyperledger#410)

* add kubernetes support

Signed-off-by: Karim TAAM <[email protected]>

* fix review issues

Signed-off-by: Karim TAAM <[email protected]>

* LogBloomCache - make sure the current segment is filled (hyperledger#411)

Make sure we cache the current cache segment with all of the data from
the beginning of the segment.  Use a flip file approach since it will be
 a partial file until done.

Signed-off-by: Danno Ferrin <[email protected]>

Co-authored-by: Abdelhamid Bakhta <[email protected]>

* Reduce recaching in Transaction Log Bloom Filter Cache (hyperledger#415)

Do a cursory cache check at start up (file is present and correct size)
instead of re-generating the cache at startup.

Signed-off-by: Danno Ferrin <[email protected]>

* fix order of nat detector (hyperledger#414)

Signed-off-by: Karim TAAM <[email protected]>

* Update SLOAD_GAS cost to 200 in Aztlan Gas Calculator (#23) (hyperledger#382)

* Update SLOAD_GAS cost to 200 in Aztlan Gas Calculator

Change SLOAD_GAS cost in Aztlan Gas Calculator from 800 to 200 and
update functions that use SLOAD_GAS.

Signed-off-by: edwardmack <[email protected]>

* Update SLOAD_GAS cost to 200 in Aztlan Gas Calculator

Change SLOAD_GAS cost in Aztlan Gas Calculator from 800 to 200 and
update functions that use SLOAD_GAS.

Signed-off-by: edwardmack <[email protected]>
Signed-off-by: Edward Mack <[email protected]>

* remove overrides

removed overrides of calculateStorageCost and
calculateStorageRefundAmount in AztlanGasCalculator because these were
causing aztlan fork not to sync with kotti testnet.

Signed-off-by: Edward Mack <[email protected]>

* merge

Signed-off-by: Edward Mack <[email protected]>

* Remove unused ExecutorService init/termination (hyperledger#419)

Signed-off-by: Horacio Mijail Anton Quiles <[email protected]>

Co-authored-by: CJ Hare <[email protected]>

* Implement Eth/64 (hyperledger#425)

Wire in the fork identifier into the status messages as Eth64.

Signed-off-by: Danno Ferrin <[email protected]>

* Adds priv_getcode (hyperledger#428)

* Adds priv_getcode

Signed-off-by: Joshua Richardson <[email protected]>

* BESU-146 - check if success and return errorResponse otherwise (hyperledger#424)

Signed-off-by: Anthony Buckle <[email protected]>

Co-authored-by: CJ Hare <[email protected]>

* fixed typos (hyperledger#429)

Signed-off-by: Sally MacFarlane <[email protected]>

Co-authored-by: CJ Hare <[email protected]>

* roll back on ALL-CAPS.md files for TSC proposal (hyperledger#376)

Signed-off-by: Felipe Faraggi <[email protected]>

* updating the changelog with 1.4.0 details (hyperledger#431)

Signed-off-by: Joshua Fernandes <[email protected]>

* [BESU-194] Remove max pivot block resets during fast sync (hyperledger#427)

* remove max pivot block resets during fast sync

* increase max retry number and fix test

* change logs in the handleFailure method

* change logs related to suspicious number of retries
Signed-off-by: Karim TAAM <[email protected]>

Co-authored-by: Danno Ferrin <[email protected]>
Co-authored-by: Jason Frame <[email protected]>
Co-authored-by: Karim T. <[email protected]>
Co-authored-by: Abdelhamid Bakhta <[email protected]>
Co-authored-by: Ratan Rai Sur <[email protected]>
Co-authored-by: mark-terry <[email protected]>
Co-authored-by: Joshua Fernandes <[email protected]>
Co-authored-by: Antoine Toulme <[email protected]>
Co-authored-by: Sally MacFarlane <[email protected]>
Co-authored-by: MadelineMurray <[email protected]>
Co-authored-by: Edward <[email protected]>
Co-authored-by: Lucas Saldanha <[email protected]>
Co-authored-by: pinges <[email protected]>
Co-authored-by: Horacio Mijail Antón Quiles <[email protected]>
Co-authored-by: CJ Hare <[email protected]>
Co-authored-by: anthonybuckle <[email protected]>
Co-authored-by: Felipe Faraggi <[email protected]>
@alexandratran alexandratran removed the doc-change-required Indicates an issue or PR that requires doc to be updated label Mar 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants