Skip to content

Commit

Permalink
Merge pull request #1562 from apache/OAK-10921
Browse files Browse the repository at this point in the history 
OAK-10921 : fixed race condition where fullGC database variables gets…
  • Loading branch information
@rishabhdaim
rishabhdaim authored Aug 13, 2024
2 parents a39580f + 7f560c6 commit dcb39ef
Show file tree
Hide file tree
Showing 3 changed files with 142 additions and 23 deletions.
73 changes: 55 additions & 18 deletions ...nt/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,9 @@ public class VersionGCRecommendations {
() -> oldestModifiedDocTimeStamp.set(0L));
oldestModifiedDocId = MIN_ID_VALUE;
log.info("fullGCTimestamp found: {}", timestampToString(oldestModifiedDocTimeStamp.get()));
// initialize the fullGC database variables i.e. fullGCTimestamp and fullGCId
setVGCSetting(of(SETTINGS_COLLECTION_FULL_GC_TIMESTAMP_PROP, oldestModifiedDocTimeStamp.get(),
SETTINGS_COLLECTION_FULL_GC_DOCUMENT_ID_PROP, oldestModifiedDocId));
} else {
oldestModifiedDocTimeStamp.set(fullGCTimestamp);
}
Expand Down Expand Up @@ -266,13 +269,16 @@ public void evaluate(VersionGCStats stats) {
long nextDuration = Math.max(precisionMs, scope.getDurationMs() / 2);
gcmon.info("Limit {} documents exceeded, reducing next collection interval to {} seconds",
this.maxCollect, TimeUnit.MILLISECONDS.toSeconds(nextDuration));
setLongSetting(VersionGarbageCollector.SETTINGS_COLLECTION_REC_INTERVAL_PROP, nextDuration);
setVGCSetting(VersionGarbageCollector.SETTINGS_COLLECTION_REC_INTERVAL_PROP, nextDuration);
stats.needRepeat = true;
} else if (!stats.canceled && !stats.ignoredGCDueToCheckPoint && !isFullGCDryRun) {
// success, we would not expect to encounter revisions older than this in the future
setLongSetting(of(SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP, scope.toMs,
SETTINGS_COLLECTION_FULL_GC_TIMESTAMP_PROP, stats.oldestModifiedDocTimeStamp));
setStringSetting(SETTINGS_COLLECTION_FULL_GC_DOCUMENT_ID_PROP, stats.oldestModifiedDocId);
setVGCSetting(SETTINGS_COLLECTION_OLDEST_TIMESTAMP_PROP, scope.toMs);

final Map<String, Object> updateVGCMap = new HashMap<>();
updateVGCMap.put(SETTINGS_COLLECTION_FULL_GC_TIMESTAMP_PROP, stats.oldestModifiedDocTimeStamp);
updateVGCMap.put(SETTINGS_COLLECTION_FULL_GC_DOCUMENT_ID_PROP, stats.oldestModifiedDocId);
updateVGCSetting(updateVGCMap);

int count = stats.deletedDocGCCount - stats.deletedLeafDocGCCount;
double usedFraction;
Expand All @@ -289,7 +295,7 @@ public void evaluate(VersionGCStats stats) {
long nextDuration = (long) Math.ceil(suggestedIntervalMs * 1.5);
log.debug("successful run using {}% of limit, raising recommended interval to {} seconds",
Math.round(usedFraction * 1000) / 10.0, TimeUnit.MILLISECONDS.toSeconds(nextDuration));
setLongSetting(VersionGarbageCollector.SETTINGS_COLLECTION_REC_INTERVAL_PROP, nextDuration);
setVGCSetting(VersionGarbageCollector.SETTINGS_COLLECTION_REC_INTERVAL_PROP, nextDuration);
} else {
log.debug("not increasing limit: collected {} documents ({}% >= {}% limit)", count, usedFraction,
allowedFraction);
Expand All @@ -304,11 +310,11 @@ public void evaluate(VersionGCStats stats) {
if (fullGCEnabled && !stats.canceled && !stats.ignoredFullGCDueToCheckPoint) {
// success, we would not expect to encounter revisions older than this in the future
if (isFullGCDryRun) {
setLongSetting(SETTINGS_COLLECTION_FULL_GC_DRY_RUN_TIMESTAMP_PROP, stats.oldestModifiedDocTimeStamp);
setStringSetting(SETTINGS_COLLECTION_FULL_GC_DRY_RUN_DOCUMENT_ID_PROP, stats.oldestModifiedDocId);
setVGCSetting(of(SETTINGS_COLLECTION_FULL_GC_DRY_RUN_TIMESTAMP_PROP, stats.oldestModifiedDocTimeStamp,
SETTINGS_COLLECTION_FULL_GC_DRY_RUN_DOCUMENT_ID_PROP, stats.oldestModifiedDocId));
} else {
setLongSetting(SETTINGS_COLLECTION_FULL_GC_TIMESTAMP_PROP, stats.oldestModifiedDocTimeStamp);
setStringSetting(SETTINGS_COLLECTION_FULL_GC_DOCUMENT_ID_PROP, stats.oldestModifiedDocId);
updateVGCSetting(of(SETTINGS_COLLECTION_FULL_GC_TIMESTAMP_PROP, stats.oldestModifiedDocTimeStamp,
SETTINGS_COLLECTION_FULL_GC_DOCUMENT_ID_PROP, stats.oldestModifiedDocId));
}

final long scopeEnd = scopeFullGC.toMs;
Expand Down Expand Up @@ -345,20 +351,51 @@ private Map<String, Object> getVGCSettings() {
return settings;
}

private void setLongSetting(String propName, long val) {
setLongSetting(of(propName, val));
/**
* Set the VGC settings with the given property and value.
* If property is not present, it will add the property to versionGC document with given value.
*
* @param propName the property name
* @param val the value
* @see VersionGCRecommendations#setVGCSetting(Map)
*/
private void setVGCSetting(final String propName, final Object val) {
final Map<String, Object> vgcMap = new HashMap<>();
vgcMap.put(propName, val);
setVGCSetting(vgcMap);
}

private void setStringSetting(String propName, String val) {
UpdateOp updateOp = new UpdateOp(SETTINGS_COLLECTION_ID, true);
updateOp.set(propName, val);
/**
* Set the VGC settings with the given properties and values.
* If properties are not present, it will add the properties to versionGC document with given values
* .
* @param propValMap the properties and values to set
*/
private void setVGCSetting(final Map<String, Object> propValMap) {
final UpdateOp updateOp = new UpdateOp(SETTINGS_COLLECTION_ID, true);
setUpdateOp(propValMap, updateOp);
vgc.getDocumentStore().createOrUpdate(Collection.SETTINGS, updateOp);
}

private void setLongSetting(final Map<String, Long> propValMap) {
UpdateOp updateOp = new UpdateOp(SETTINGS_COLLECTION_ID, true);
propValMap.forEach(updateOp::set);
vgc.getDocumentStore().createOrUpdate(Collection.SETTINGS, updateOp);
private void setUpdateOp(final Map<String, Object> propValMap, final UpdateOp updateOp) {
propValMap.forEach((k, v) -> {
if (v instanceof Number) updateOp.set(k, ((Number) v).longValue());
if (v instanceof String) updateOp.set(k, (String) v);
if (v instanceof Boolean) updateOp.set(k, (Boolean) v);
});
}

/**
* Update the VGC settings with the given properties and values.
* Properties are only updated if they already exists in the versionGC document.
*
* @param propValMap the properties and values to update
*/
private void updateVGCSetting(final Map<String, Object> propValMap) {
final UpdateOp updateOp = new UpdateOp(SETTINGS_COLLECTION_ID, false);
setUpdateOp(propValMap, updateOp);
propValMap.forEach((k, v) -> updateOp.contains(k, true));
vgc.getDocumentStore().findAndUpdate(Collection.SETTINGS, updateOp);
}

@NotNull
Expand Down
7 changes: 3 additions & 4 deletions ...-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCInitTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ public void lazyInitialize() throws Exception {

vgc = store.find(SETTINGS, SETTINGS_COLLECTION_ID);
assertNotNull(vgc);
assertEquals(0L, vgc.get(SETTINGS_COLLECTION_FULL_GC_TIMESTAMP_PROP));

// fullGC values shouldn't have been updated without fullGC enabled
assertNull(vgc.get(SETTINGS_COLLECTION_FULL_GC_TIMESTAMP_PROP));
assertNull(vgc.get(SETTINGS_COLLECTION_FULL_GC_DOCUMENT_ID_PROP));
}

Expand Down Expand Up @@ -124,9 +126,6 @@ public void lazyInitializeWithFullGCDryRun() throws Exception {
vgc = store.find(SETTINGS, SETTINGS_COLLECTION_ID);
assertNotNull(vgc);
// fullGC values shouldn't have been updated in dryRun mode
System.out.println(stats.oldestModifiedDocId);
System.out.println(stats.oldestModifiedDocTimeStamp);
System.out.println(vgc);
assertNull(vgc.get(SETTINGS_COLLECTION_FULL_GC_TIMESTAMP_PROP));
assertNull(vgc.get(SETTINGS_COLLECTION_FULL_GC_DOCUMENT_ID_PROP));

Expand Down
85 changes: 84 additions & 1 deletion ...t/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.junit.Assume.assumeThat;
import static org.junit.Assume.assumeTrue;

import org.apache.jackrabbit.guava.common.cache.Cache;
Expand Down Expand Up @@ -1611,6 +1610,90 @@ private Iterator<NodeDocument> candidates(long fromModified, long toModified, in

// OAK-10199 END

// OAK-10921
@Test
public void resetGCFromOakRunWhileRunning() throws Exception {
// if we reset fullGC from any external source while GC is running,
// it should not update the fullGC variables.
resetFullGCExternally(false);
}

@Test
public void resetFullGCFromOakRunWhileRunning() throws Exception {
// if we reset fullGC from any external source while GC is running,
// it should not update the fullGC variables.
resetFullGCExternally(true);
}

private void resetFullGCExternally(final boolean resetFullGCOnly) throws Exception {
//1. Create nodes with properties
NodeBuilder b1 = store1.getRoot().builder();

// Add property to node & save
for (int i = 0; i < 5; i++) {
b1.child("z"+i).setProperty("prop", "foo", STRING);
}
store1.merge(b1, EmptyHook.INSTANCE, CommitInfo.EMPTY);
store1.runBackgroundOperations();

// enable the full gc flag
writeField(gc, "fullGCEnabled", true, true);
long maxAge = 1; //hours
long delta = MINUTES.toMillis(10);
//1. Go past GC age and check no GC done as nothing deleted
clock.waitUntil(getCurrentTimestamp() + maxAge);
VersionGCStats stats = gc(gc, maxAge, HOURS);
assertStatsCountsZero(stats);

//Remove property
NodeBuilder b2 = store1.getRoot().builder();
for (int i = 0; i < 5; i++) {
b2.getChildNode("z"+i).removeProperty("prop");
}
store1.merge(b2, EmptyHook.INSTANCE, CommitInfo.EMPTY);
store1.runBackgroundOperations();

final AtomicReference<VersionGarbageCollector> gcRef = Atomics.newReference();
final VersionGCSupport gcSupport = new VersionGCSupport(store1.getDocumentStore()) {

@Override
public Iterable<NodeDocument> getModifiedDocs(long fromModified, long toModified, int limit, @NotNull String fromId,
final @NotNull Set<String> includePaths, final @NotNull Set<String> excludePaths) {
// reset fullGC variables externally while GC is running
if (resetFullGCOnly) {
gcRef.get().resetFullGC();
} else {
gcRef.get().reset();
}
return super.getModifiedDocs(fromModified, toModified, limit, fromId, includePaths, excludePaths);
}
};

gcRef.set(new VersionGarbageCollector(store1, gcSupport, true, false, false, 3));

//3. Check that deleted property does get collected post maxAge
clock.waitUntil(clock.getTime() + HOURS.toMillis(maxAge*2) + delta);

Document document = store1.getDocumentStore().find(SETTINGS, SETTINGS_COLLECTION_ID);
assert document != null;
assertNotNull(document.get(SETTINGS_COLLECTION_FULL_GC_TIMESTAMP_PROP));
assertNotNull(document.get(SETTINGS_COLLECTION_FULL_GC_DOCUMENT_ID_PROP));

stats = gcRef.get().gc(maxAge*2, HOURS);

document = store1.getDocumentStore().find(SETTINGS, SETTINGS_COLLECTION_ID);
assertEquals(5, stats.updatedFullGCDocsCount);
assertEquals(5, stats.deletedPropsCount);
assertEquals(MIN_ID_VALUE, stats.oldestModifiedDocId);

// 4. verify that fullGC variables are not updated after resetting them
assert document != null;
assertNull(document.get(SETTINGS_COLLECTION_FULL_GC_TIMESTAMP_PROP));
assertNull(document.get(SETTINGS_COLLECTION_FULL_GC_DOCUMENT_ID_PROP));
}

// OAK-10921 END

@SuppressWarnings("unchecked")
/**
* Test when a revision on a parent is becoming garbage on a property
Expand Down

0 comments on commit dcb39ef

Please sign in to comment.