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

Fix endless chunk rewriting when db is idle - improvement #4000

Closed
wants to merge 4 commits into from
Closed

Fix endless chunk rewriting when db is idle - improvement #4000

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c83b88d
Adapt chunk rewriting/compaction stop condition in store housekeeping
vreuland Feb 4, 2024
2e860ac
Merge branch 'h2database:master' into fix-endless-chunk-rewriting-whe…
vreuland Feb 12, 2024
cab7c06
Refine chunk rewriting/compaction stop condition in store housekeeping
vreuland Feb 22, 2024
adac0f9
Merge remote-tracking branch 'origin/fix-endless-chunk-rewriting-when…
vreuland Feb 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 17 additions & 9 deletions h2/src/main/org/h2/mvstore/RandomAccessStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,10 @@ public abstract class RandomAccessStore extends FileStore<SFChunk>

private long reservedLow;
private long reservedHigh;
private boolean stopIdleHousekeeping;

/** Used to determine when to stop chunk rewriting housekeeping */
private boolean lastHousekeepingHasRewrittenChunks;
private int previousChunksFillRate = -1;

public RandomAccessStore(Map<String, Object> config) {
super(config);
Expand Down Expand Up @@ -702,10 +705,6 @@ private void shrinkIfPossible(int minPercent) {

@Override
protected void doHousekeeping(MVStore mvStore) throws InterruptedException {
boolean idle = isIdle();
if (idle && stopIdleHousekeeping) {
return;
}
int autoCommitMemory = mvStore.getAutoCommitMemory();
int fillRate = getFillRate();
if (isFragmented() && fillRate < getAutoCompactFillRate()) {
Expand All @@ -719,22 +718,31 @@ protected void doHousekeeping(MVStore mvStore) throws InterruptedException {
});
}

int chunksFillRate = getRewritableChunksFillRate();
int adjustedChunksFillRate = 100 - (100 - chunksFillRate) / 2;
int fillRateToCompare = isIdle() ? chunksFillRate : adjustedChunksFillRate;
int chunksFillRate = getChunksFillRate();
if (lastHousekeepingHasRewrittenChunks && chunksFillRate == previousChunksFillRate) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this condition may yield true only by a coincident, because if last housekeeping has rewritten chunks, chunks fill rate is likely to change, even without any user activity

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be honest, It is based on the observations I have made on my database where practically the chunkFillRate was not moving during the housekeeping (quickly stayed in my case at 78%): Only the rewriteable chunk fill rate was incrementally increasing (up to 100%) until the retention time of the first rewritten chunks was hit, driving back the rewritable chunk fill rate to the starting point and restarting the useless housekeeping cycle.
The condition has also an equality ( and not a <=) because I needed a way to detect when to "resume" the housekeeping if some user operations were made (at least enough operations to have an impact on the chunkFillRate) and without using isIdle.
So the logic is really:

  1. The housekeeping has just rewritten some chunk with no impact on the chunkFillRate ? -> Yes, then stop performing the housekeeping
  2. When to resume the housekeeping? -> Whenever there was enough user operations to change the chunkFillRate (by at least a point)

But you are right, I guess the fact that the chunkFillRate is not "moved" by the housekeeping in my tests might very well be specific to my database(s). And depending on the database, we could probably have a situation where chunkFillRate is also somehow fluctuating (78, 79, 77, 78, 79, ...) after each housekeeping, blowing up the stopping condition.

I have seen you have made another proposal (in #4006). Thanks a lot for that. I will thoroughly test it and will come back to you as soon as I can.

// Do not perform chunks rewriting if there is no progress or change since last chunk rewrite
// This is to avoid endless chunks rewriting in certain cases (cf. issue #3909)
return;
}
previousChunksFillRate = chunksFillRate;
lastHousekeepingHasRewrittenChunks = false;

int rewritableChunksFillRate = getRewritableChunksFillRate();
int adjustedChunksFillRate = 100 - (100 - rewritableChunksFillRate) / 2;
int fillRateToCompare = isIdle() ? rewritableChunksFillRate : adjustedChunksFillRate;
if (fillRateToCompare < getTargetFillRate()) {
mvStore.tryExecuteUnderStoreLock(() -> {
int writeLimit = autoCommitMemory;
if (!isIdle()) {
writeLimit /= 4;
}
if (rewriteChunks(writeLimit, isIdle() ? adjustedChunksFillRate : chunksFillRate)) {
lastHousekeepingHasRewrittenChunks = true;
dropUnusedChunks();
}
return true;
});
}
stopIdleHousekeeping = idle && getFillRate() <= fillRate && getRewritableChunksFillRate() <= chunksFillRate;
}

private int getTargetFillRate() {
Expand Down