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

[Merged by Bors] - sync2: fptree: don't get next in FingerprintInterval if not needed #6558

Closed
wants to merge 3 commits into from

Conversation

ivan4th
Copy link
Contributor

@ivan4th ivan4th commented Dec 19, 2024

Motivation

Retrieving the next item (one immediately following the fingerprinted interval) when fingerprinting an interval is only useful during slow splitting, when we need to split a range but "easy split" (involving only in-memory FPTree) fails. In other cases, the next item is not really needed and getting it may incur an unwanted database access.

TestFPTreeManyItems fails intermittently when it happens to fingerprint an empty range during random testing due to a problem with handling next items.

Description

This removes unneeded retrieval of the next item and fixes TestFPTreeManyItems.

Retrieving the next item when fingerprinting an interval is only
useful during slow splitting, when we need to split a range but "easy
split" (involving only in-memory FPTree) fails. In other cases,
the next item is not really needed and getting it may incur an
unwanted database access.

This also fixes intermittent failures of TestFPTreeManyItems test.
sync2/fptree/export_test.go Outdated Show resolved Hide resolved
Copy link

codecov bot commented Dec 19, 2024

Codecov Report

Attention: Patch coverage is 95.65217% with 1 line in your changes missing coverage. Please review.

Project coverage is 79.8%. Comparing base (febfe27) to head (0ae8a25).
Report is 2 commits behind head on develop.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
sync2/fptree/fptree.go 95.6% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           develop   #6558   +/-   ##
=======================================
  Coverage     79.8%   79.8%           
=======================================
  Files          354     354           
  Lines        47015   47023    +8     
=======================================
+ Hits         37534   37556   +22     
+ Misses        7351    7343    -8     
+ Partials      2130    2124    -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ivan4th
Copy link
Contributor Author

ivan4th commented Dec 19, 2024

bors merge

spacemesh-bors bot pushed a commit that referenced this pull request Dec 19, 2024
…6558)

## Motivation

Retrieving the next item (one immediately following the fingerprinted interval) when fingerprinting an interval is only useful during slow splitting, when we need to split a range but "easy split" (involving only in-memory FPTree) fails. In other cases, the next item is not really needed and getting it may incur an unwanted database access.

`TestFPTreeManyItems` fails intermittently when it happens to fingerprint an empty range during random testing due to a problem with handling next items.
@spacemesh-bors
Copy link

Build failed:

@ivan4th
Copy link
Contributor Author

ivan4th commented Dec 19, 2024

Unrelated TestPartition_50_50 failure:

    logger.go:146: 2024-12-19T21:25:05.431Z	ERROR	TestPartition_50_50	tests/partition_test.go:175	client state differs from ref state	***"client": "boot-1", "ref_client": "boot-0", "layer": 23, "client_hash": "af1349b9f5f9a1a6a0404dea36dcc9499bcb25c9adc112b7cc9a93cae41f3262", "ref_hash": "0000000000000000000000000000000000000000000000000000000000000000"***
    logger.go:146: 2024-12-19T21:25:05.431Z	ERROR	TestPartition_50_50	tests/partition_test.go:175	client state differs from ref state	***"client": "smesher-0", "ref_client": "boot-0", "layer": 23, "client_hash": "af1349b9f5f9a1a6a0404dea36dcc9499bcb25c9adc112b7cc9a93cae41f3262", "ref_hash": "0000000000000000000000000000000000000000000000000000000000000000"***
    logger.go:146: 2024-12-19T21:25:05.431Z	ERROR	TestPartition_50_50	tests/partition_test.go:175	client state differs from ref state	***"client": "smesher-1", "ref_client": "boot-0", "layer": 23, "client_hash": "af1349b9f5f9a1a6a0404dea36dcc9499bcb25c9adc112b7cc9a93cae41f3262", "ref_hash": "0000000000000000000000000000000000000000000000000000000000000000"***

@ivan4th
Copy link
Contributor Author

ivan4th commented Dec 19, 2024

bors merge

spacemesh-bors bot pushed a commit that referenced this pull request Dec 19, 2024
…6558)

## Motivation

Retrieving the next item (one immediately following the fingerprinted interval) when fingerprinting an interval is only useful during slow splitting, when we need to split a range but "easy split" (involving only in-memory FPTree) fails. In other cases, the next item is not really needed and getting it may incur an unwanted database access.

`TestFPTreeManyItems` fails intermittently when it happens to fingerprint an empty range during random testing due to a problem with handling next items.
@spacemesh-bors
Copy link

Pull request successfully merged into develop.

Build succeeded:

@spacemesh-bors spacemesh-bors bot changed the title sync2: fptree: don't get next in FingerprintInterval if not needed [Merged by Bors] - sync2: fptree: don't get next in FingerprintInterval if not needed Dec 19, 2024
@spacemesh-bors spacemesh-bors bot closed this Dec 19, 2024
@spacemesh-bors spacemesh-bors bot deleted the sync2/no-next branch December 19, 2024 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants