diff --git a/.idea/vcs.xml b/.idea/vcs.xml index 48557884a8893..c668657daf908 100644 --- a/.idea/vcs.xml +++ b/.idea/vcs.xml @@ -1,20 +1,20 @@ - - - + + + - + - + \ No newline at end of file diff --git a/server/src/main/java/org/opensearch/common/cache/tier/DiskTierTookTimePolicy.java b/server/src/main/java/org/opensearch/common/cache/tier/DiskTierTookTimePolicy.java index f5c52b5f0afd8..15044ac603c5a 100644 --- a/server/src/main/java/org/opensearch/common/cache/tier/DiskTierTookTimePolicy.java +++ b/server/src/main/java/org/opensearch/common/cache/tier/DiskTierTookTimePolicy.java @@ -54,18 +54,23 @@ protected void setThreshold(TimeValue threshold) { // protected so that we can m @Override public boolean checkData(BytesReference data) { - if (threshold.equals(TimeValue.ZERO)) { - return true; - } Long tookTimeNanos; try { tookTimeNanos = getPolicyInfoFn.apply(data).getTookTimeNanos(); } catch (Exception e) { - // If we can't retrieve the took time for whatever reason, admit the data to be safe - return true; + // If we can't read a CachePolicyInfoWrapper from the BytesReference, reject the data + return false; } + if (tookTimeNanos == null) { - // Received a null took time -> this QSR is from an old version which does not have took time, we should accept it + // If the wrapper contains null took time, reject the data + // This can happen if no CachePolicyInfoWrapper was written to the BytesReference, as the wrapper's constructor + // reads an optional long, which will end up as null in this case. This is why we should reject it. + return false; + } + + if (threshold.equals(TimeValue.ZERO)) { + // If the policy is set to zero, admit any well-formed data return true; } TimeValue tookTime = TimeValue.timeValueNanos(tookTimeNanos); diff --git a/server/src/test/java/org/opensearch/common/cache/tier/DiskTierTookTimePolicyTests.java b/server/src/test/java/org/opensearch/common/cache/tier/DiskTierTookTimePolicyTests.java index 31333c3c0a9d0..5dbc2d2706091 100644 --- a/server/src/test/java/org/opensearch/common/cache/tier/DiskTierTookTimePolicyTests.java +++ b/server/src/test/java/org/opensearch/common/cache/tier/DiskTierTookTimePolicyTests.java @@ -83,6 +83,38 @@ public void testTookTimePolicy() throws Exception { assertTrue(longResult); } + public void testMissingWrapper() throws Exception { + DiskTierTookTimePolicy tookTimePolicy = getTookTimePolicy(); + tookTimePolicy.setThreshold(TimeValue.ZERO); + QuerySearchResult qsr = getQSR(); + BytesStreamOutput out = new BytesStreamOutput(); + qsr.writeTo(out); + BytesReference missingWrapper = out.bytes(); + boolean allowedMissingWrapper = tookTimePolicy.checkData(missingWrapper); + assertFalse(allowedMissingWrapper); + } + + public void testNullTookTime() throws Exception { + // Null took time should always be rejected (because it might be the result of a + // BytesReference without a CachePolicyInfoWrapper in front of it) + + DiskTierTookTimePolicy zeroThreshold = getTookTimePolicy(); + zeroThreshold.setThreshold(TimeValue.ZERO); + DiskTierTookTimePolicy nonZeroThreshold = getTookTimePolicy(); + nonZeroThreshold.setThreshold(new TimeValue(10L)); + + Long nullTookTime = null; + CachePolicyInfoWrapper nullWrapper = new CachePolicyInfoWrapper(nullTookTime); + BytesStreamOutput out = new BytesStreamOutput(); + nullWrapper.writeTo(out); + QuerySearchResult qsr = getQSR(); + qsr.writeTo(out); + BytesReference data = out.bytes(); + + assertFalse(zeroThreshold.checkData(data)); + assertFalse(nonZeroThreshold.checkData(data)); + } + public static QuerySearchResult getQSR() { // package-private, also used by IndicesRequestCacheTests.java // setup from QuerySearchResultTests.java