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

fixes #1920 #2049

Open
wants to merge 23 commits into
base: integration
Choose a base branch
from
Open
Changes from 3 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
431739c
fixes #1920
ivakegg Jul 28, 2023
e855207
formatting
ivakegg Aug 1, 2023
c22462e
Merge branch 'integration' into bugfix/DATAWAVE-1920
ivakegg Aug 9, 2023
dd48968
Merge branch 'integration' into bugfix/DATAWAVE-1920
ivakegg Sep 20, 2023
77a195b
Merge branch 'integration' into bugfix/DATAWAVE-1920
ivakegg Sep 29, 2023
bfce3f0
Merge branch 'integration' into bugfix/DATAWAVE-1920
ivakegg Oct 3, 2023
5aaeac1
Merge branch 'integration' into bugfix/DATAWAVE-1920
ivakegg Oct 4, 2023
bbe0978
Merge branch 'integration' into bugfix/DATAWAVE-1920
ivakegg Oct 25, 2023
e133d1f
Merge branch 'integration' into bugfix/DATAWAVE-1920
ivakegg Nov 24, 2023
61ca815
Merge branch 'integration' into bugfix/DATAWAVE-1920
ivakegg Nov 24, 2023
9cc16d8
Merge branch 'integration' into bugfix/DATAWAVE-1920
ivakegg Dec 16, 2023
cd96f88
Merge branch 'integration' into bugfix/DATAWAVE-1920
ivakegg Dec 20, 2023
a1b7de9
Merge branch 'integration' into bugfix/DATAWAVE-1920
ivakegg Jan 16, 2024
1bf0c3d
Merge branch 'integration' into bugfix/DATAWAVE-1920
ivakegg Jan 26, 2024
f49f251
Merge branch 'integration' into bugfix/DATAWAVE-1920
ivakegg Feb 9, 2024
d686d67
Merge branch 'integration' into bugfix/DATAWAVE-1920
ivakegg Feb 14, 2024
ada485a
Merge branch 'integration' into bugfix/DATAWAVE-1920
ivakegg Feb 16, 2024
53e80aa
Merge branch 'integration' into bugfix/DATAWAVE-1920
ivakegg Apr 5, 2024
70605f1
Merge branch 'integration' into bugfix/DATAWAVE-1920
ivakegg Apr 12, 2024
3b478ae
Merge branch 'integration' into bugfix/DATAWAVE-1920
ivakegg May 3, 2024
3f1fbdb
Merge branch 'integration' into bugfix/DATAWAVE-1920
ivakegg May 10, 2024
4a8da93
Merge branch 'integration' into bugfix/DATAWAVE-1920
ivakegg May 10, 2024
6577e0f
Merge branch 'integration' into bugfix/DATAWAVE-1920
ivakegg Jan 6, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -814,6 +814,9 @@ else if (key.compareTo(this.lastRangeSeeked.getStartKey()) < 0) {
}

// if we have any persisted data or we have scanned a significant number of keys, then persist it completely
// Note that we will not persist the data if we are not over the scan threshold and the data fits into
// one buffer (no persisted data). That means that if we need to rebuild the sorted set that we can do
// it relatively quickly so no need for a persisted set with a complete marker.
Comment on lines +818 to +820
Copy link
Collaborator

@jwomeara jwomeara Aug 10, 2023

Choose a reason for hiding this comment

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

I'm not sure what this comment has to do with the original issue I reported.

In the scenario I described, one of two things happen.

Either,

  1. We have entries buffered in memory, and that triggers the ivarator to persist the entries to disk and create a complete file.

OR

  1. We have no entries buffered in memory (because everything was ALREADY persisted to disk), and that does NOT trigger the creation of a complete file.

I don't think this has anything to do with the scan threshold. If it did, then in scenario #1 the code wouldn't persist the buffer to disk, (but it absolutely does!!!). Scenario #1 is not the problem.

Scenario #2 where data has already been written to disk, but there are no results in the in-memory buffer is the problem. We already have data written to disk, but we aren't creating the complete file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am pretty sure that "this.set.hasPersistedData()" is the part of the check that takes care of your scenario 2.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@jwomeara are you saying that the code in scenario 2 is not making an accurate determination about the sate of persistence? That it's not enough that the buffer is null to call it "persisted". Should the check be buffer == null && isPersisted instead of an || ? Or am I misunderstanding altogether?

if (this.set != null && (this.set.hasPersistedData() || (scanThreshold <= scannedKeys.get()))) {
forcePersistence();
}
Expand Down