Skip to content

Commit

Permalink
Revert "OAK-11184 : introduce prevNoProp cache to avoid expensive pre…
Browse files Browse the repository at this point in the history
…v docs s…"

This reverts commit 1c1825b.
  • Loading branch information
stefan-egli authored Nov 6, 2024
1 parent f54b4cf commit 42e7960
Show file tree
Hide file tree
Showing 16 changed files with 28 additions and 687 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,7 @@ static DocumentNodeStore configureDocumentMk(Options options,
docStoreOpts.getNodeCachePercentage(),
docStoreOpts.getPrevDocCachePercentage(),
docStoreOpts.getChildrenCachePercentage(),
docStoreOpts.getDiffCachePercentage(),
docStoreOpts.getPrevNoPropCachePercentage()
docStoreOpts.getDiffCachePercentage()
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ public class DocumentNodeStoreOptions implements OptionsBean {
private final OptionSpec<Integer> prevDocCachePercentage;
private final OptionSpec<Integer> childrenCachePercentage;
private final OptionSpec<Integer> diffCachePercentage;
private final OptionSpec<Integer> prevNoPropCachePercentage;
private OptionSet options;

public DocumentNodeStoreOptions(OptionParser parser){
Expand All @@ -57,9 +56,6 @@ public DocumentNodeStoreOptions(OptionParser parser){
diffCachePercentage = parser.
accepts("diffCachePercentage", "Percentage of cache to be allocated towards Diff cache")
.withRequiredArg().ofType(Integer.class).defaultsTo(30);
prevNoPropCachePercentage = parser.
accepts("prevNoPropCachePercentage", "Percentage of cache to be allocated towards PrevNoProp cache")
.withRequiredArg().ofType(Integer.class).defaultsTo(1);
}

@Override
Expand Down Expand Up @@ -116,10 +112,6 @@ public int getDiffCachePercentage() {
return diffCachePercentage.value(options);
}

public int getPrevNoPropCachePercentage() {
return prevNoPropCachePercentage.value(options);
}

public boolean isCacheDistributionDefined(){
return options.has(nodeCachePercentage) ||
options.has(prevDocCachePercentage) ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,7 @@ private void configureCacheForReadOnlyMode(DocumentNodeStoreBuilder<?> builder)
35,
10,
15,
2,
1
2
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,6 @@
description = "Percentage of cache to be allocated towards Diff cache")
int diffCachePercentage() default DEFAULT_DIFF_CACHE_PERCENTAGE;

@AttributeDefinition(
name = "PrevNoProp Cache",
description = "Percentage of cache to be allocated towards PrevNoProp cache."
+ " This cache is used to keep non existence of properties in previous documents and can be small.")
int prevNoPropCachePercentage() default DEFAULT_PREV_DOC_CACHE_PERCENTAGE;

@AttributeDefinition(
name = "LIRS Cache Segment Count",
description = "The number of segments in the LIRS cache " +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@
import org.apache.jackrabbit.oak.plugins.document.util.LeaseCheckDocumentStoreWrapper;
import org.apache.jackrabbit.oak.plugins.document.util.LoggingDocumentStoreWrapper;
import org.apache.jackrabbit.oak.plugins.document.util.ReadOnlyDocumentStoreWrapperFactory;
import org.apache.jackrabbit.oak.plugins.document.util.StringValue;
import org.apache.jackrabbit.oak.plugins.document.util.ThrottlingDocumentStoreWrapper;
import org.apache.jackrabbit.oak.plugins.document.util.TimingDocumentStoreWrapper;
import org.apache.jackrabbit.oak.plugins.document.util.Utils;
Expand Down Expand Up @@ -440,12 +439,6 @@ public final class DocumentNodeStore
*/
private final DiffCache diffCache;

/**
* Tiny cache for non existence of any revisions in previous documents
* for particular properties.
*/
private final Cache<StringValue, StringValue> prevNoPropCache;

/**
* The commit value resolver for this node store.
*/
Expand Down Expand Up @@ -690,9 +683,6 @@ public int getMemory() {

diffCache = builder.getDiffCache(this.clusterId);

// builder checks for feature toggle directly and returns null if disabled
prevNoPropCache = builder.buildPrevNoPropCache();

// check if root node exists
NodeDocument rootDoc = store.find(NODES, Utils.getIdFromPath(ROOT));
if (rootDoc == null) {
Expand Down Expand Up @@ -870,13 +860,6 @@ public int getMemory() {
LOG.info("Initialized DocumentNodeStore with clusterNodeId: {}, updateLimit: {} ({})",
clusterId, updateLimit,
getClusterNodeInfoDisplayString());
if (prevNoPropCache == null) {
LOG.info("prevNoProp cache is disabled");
} else {
// unfortunate that the guava cache doesn't unveil its max size
// hence falling back to using the builder's original value for now.
LOG.info("prevNoProp cache is enabled with size: " + builder.getPrevNoPropCacheSize());
}

if (!builder.isBundlingDisabled()) {
bundlingConfigHandler.initialize(this, executor);
Expand Down Expand Up @@ -1323,10 +1306,6 @@ public Predicate<Path> getNodeCachePredicate() {
return nodeCachePredicate;
}

public Cache<StringValue, StringValue> getPrevNoPropCache() {
return prevNoPropCache;
}

/**
* Returns the journal entry that will be stored in the journal with the
* next background updated.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,7 @@ public class DocumentNodeStoreBuilder<T extends DocumentNodeStoreBuilder<T>> {
private static final Logger LOG = LoggerFactory.getLogger(DocumentNodeStoreBuilder.class);

public static final long DEFAULT_MEMORY_CACHE_SIZE = 256 * 1024 * 1024;
public static final int DEFAULT_NODE_CACHE_PERCENTAGE = 34;
public static final int DEFAULT_PREV_NO_PROP_CACHE_PERCENTAGE = 1;
public static final int DEFAULT_NODE_CACHE_PERCENTAGE = 35;
public static final int DEFAULT_PREV_DOC_CACHE_PERCENTAGE = 4;
public static final int DEFAULT_CHILDREN_CACHE_PERCENTAGE = 15;
public static final int DEFAULT_DIFF_CACHE_PERCENTAGE = 30;
Expand Down Expand Up @@ -135,12 +134,10 @@ public class DocumentNodeStoreBuilder<T extends DocumentNodeStoreBuilder<T>> {
private Feature cancelInvalidationFeature;
private Feature docStoreFullGCFeature;
private Feature docStoreEmbeddedVerificationFeature;
private Feature prevNoPropCacheFeature;
private Weigher<CacheValue, CacheValue> weigher = new EmpiricalWeigher();
private long memoryCacheSize = DEFAULT_MEMORY_CACHE_SIZE;
private int nodeCachePercentage = DEFAULT_NODE_CACHE_PERCENTAGE;
private int prevDocCachePercentage = DEFAULT_PREV_DOC_CACHE_PERCENTAGE;
private int prevNoPropCachePercentage = DEFAULT_PREV_NO_PROP_CACHE_PERCENTAGE;
private int childrenCachePercentage = DEFAULT_CHILDREN_CACHE_PERCENTAGE;
private int diffCachePercentage = DEFAULT_DIFF_CACHE_PERCENTAGE;
private int cacheSegmentCount = DEFAULT_CACHE_SEGMENT_COUNT;
Expand Down Expand Up @@ -454,16 +451,6 @@ public Feature getDocStoreEmbeddedVerificationFeature() {
return docStoreEmbeddedVerificationFeature;
}

public T setPrevNoPropCacheFeature(@Nullable Feature prevNoPropCacheFeature) {
this.prevNoPropCacheFeature = prevNoPropCacheFeature;
return thisBuilder();
}

@Nullable
public Feature getPrevNoPropCacheFeature() {
return prevNoPropCacheFeature;
}

public T setLeaseFailureHandler(LeaseFailureHandler leaseFailureHandler) {
this.leaseFailureHandler = leaseFailureHandler;
return thisBuilder();
Expand Down Expand Up @@ -599,20 +586,17 @@ public T memoryCacheSize(long memoryCacheSize) {
public T memoryCacheDistribution(int nodeCachePercentage,
int prevDocCachePercentage,
int childrenCachePercentage,
int diffCachePercentage,
int prevNoPropCachePercentage) {
int diffCachePercentage) {
checkArgument(nodeCachePercentage >= 0);
checkArgument(prevDocCachePercentage >= 0);
checkArgument(childrenCachePercentage>= 0);
checkArgument(diffCachePercentage >= 0);
checkArgument(prevNoPropCachePercentage >= 0);
checkArgument(nodeCachePercentage + prevDocCachePercentage + childrenCachePercentage +
diffCachePercentage + prevNoPropCachePercentage < 100);
diffCachePercentage < 100);
this.nodeCachePercentage = nodeCachePercentage;
this.prevDocCachePercentage = prevDocCachePercentage;
this.childrenCachePercentage = childrenCachePercentage;
this.diffCachePercentage = diffCachePercentage;
this.prevNoPropCachePercentage = prevNoPropCachePercentage;
return thisBuilder();
}

Expand All @@ -624,21 +608,13 @@ public long getPrevDocumentCacheSize() {
return memoryCacheSize * prevDocCachePercentage / 100;
}

public long getPrevNoPropCacheSize() {
// feature toggle overrules the prevNoPropCachePercentage config
if (!isPrevNoPropCacheEnabled()) {
return 0;
}
return memoryCacheSize * prevNoPropCachePercentage / 100;
}

public long getChildrenCacheSize() {
return memoryCacheSize * childrenCachePercentage / 100;
}

public long getDocumentCacheSize() {
return memoryCacheSize - getNodeCacheSize() - getPrevDocumentCacheSize() - getChildrenCacheSize()
- getDiffCacheSize() - getPrevNoPropCacheSize();
- getDiffCacheSize();
}

public long getDiffCacheSize() {
Expand Down Expand Up @@ -922,30 +898,6 @@ public NodeDocumentCache buildNodeDocumentCache(DocumentStore docStore, NodeDocu
return new NodeDocumentCache(nodeDocumentsCache, nodeDocumentsCacheStats, prevDocumentsCache, prevDocumentsCacheStats, locks);
}

/**
* Checks the feature toggle for prevNoProp cache and returns true if that's enabled
* @return true if the prevNoProp feature toggle is enabled, false otherwise
*/
private boolean isPrevNoPropCacheEnabled() {
final Feature prevNoPropFeature = getPrevNoPropCacheFeature();
return prevNoPropFeature != null && prevNoPropFeature.isEnabled();
}

/**
* Builds the prevNoProp cache, if the corresponding feature toggle is enabled.
* Returns null otherwise
* @return null if prevNoProp feature is disabled and size non-null, a newly built cache otherwise
*/
@Nullable
public Cache<StringValue, StringValue> buildPrevNoPropCache() {
// if feature toggle is off or the config is explicitly set to 0, then no cache
if (!isPrevNoPropCacheEnabled() || getPrevNoPropCacheSize() == 0) {
return null;
}
// no persistent cache for now as this is only a tiny cache
return buildCache("PREV_NOPROP", getPrevNoPropCacheSize(), new CopyOnWriteArraySet<>());
}

/**
* @deprecated Use {@link #setNodeCachePathPredicate(Predicate)} instead.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,12 +201,6 @@ public class DocumentNodeStoreService {
*/
private static final String FT_NAME_EMBEDDED_VERIFICATION = "FT_EMBEDDED_VERIFICATION_OAK-10633";

/**
* Feature toggle name to enable the prev-no-prop cache.
* prev-no-prop refers to previous document not containing a particular property key.
*/
private static final String FT_NAME_PREV_NO_PROP_CACHE = "FT_PREV_NO_PROP_OAK-11184";

// property name constants - values can come from framework properties or OSGi config
public static final String CUSTOM_BLOB_STORE = "customBlobStore";
public static final String PROP_REV_RECOVERY_INTERVAL = "lastRevRecoveryJobIntervalInSecs";
Expand Down Expand Up @@ -246,7 +240,6 @@ static DocumentStoreType fromString(String type) {
private Feature cancelInvalidationFeature;
private Feature docStoreFullGCFeature;
private Feature docStoreEmbeddedVerificationFeature;
private Feature prevNoPropCacheFeature;
private ComponentContext context;
private Whiteboard whiteboard;
private long deactivationTimestamp = 0;
Expand Down Expand Up @@ -285,7 +278,6 @@ protected void activate(ComponentContext context, Configuration config) throws E
cancelInvalidationFeature = Feature.newFeature(FT_NAME_CANCEL_INVALIDATION, whiteboard);
docStoreFullGCFeature = Feature.newFeature(FT_NAME_FULL_GC, whiteboard);
docStoreEmbeddedVerificationFeature = Feature.newFeature(FT_NAME_EMBEDDED_VERIFICATION, whiteboard);
prevNoPropCacheFeature = Feature.newFeature(FT_NAME_PREV_NO_PROP_CACHE, whiteboard);

registerNodeStoreIfPossible();
}
Expand Down Expand Up @@ -495,8 +487,7 @@ private void configureBuilder(DocumentNodeStoreBuilder<?> builder) {
config.nodeCachePercentage(),
config.prevDocCachePercentage(),
config.childrenCachePercentage(),
config.diffCachePercentage(),
config.prevNoPropCachePercentage()).
config.diffCachePercentage()).
setCacheSegmentCount(config.cacheSegmentCount()).
setCacheStackMoveDistance(config.cacheStackMoveDistance()).
setBundlingDisabled(config.bundlingDisabled()).
Expand All @@ -508,7 +499,6 @@ private void configureBuilder(DocumentNodeStoreBuilder<?> builder) {
setCancelInvalidationFeature(cancelInvalidationFeature).
setDocStoreFullGCFeature(docStoreFullGCFeature).
setDocStoreEmbeddedVerificationFeature(docStoreEmbeddedVerificationFeature).
setPrevNoPropCacheFeature(prevNoPropCacheFeature).
setThrottlingEnabled(config.throttlingEnabled()).
setFullGCEnabled(config.fullGCEnabled()).
setFullGCIncludePaths(config.fullGCIncludePaths()).
Expand Down Expand Up @@ -680,10 +670,6 @@ protected void deactivate() {
docStoreEmbeddedVerificationFeature.close();
}

if (prevNoPropCacheFeature != null) {
prevNoPropCacheFeature.close();
}

unregisterNodeStore();
}

Expand Down
Loading

0 comments on commit 42e7960

Please sign in to comment.