mikemccand commented on code in PR #13206:
URL: https://github.com/apache/lucene/pull/13206#discussion_r1539130332
##########
lucene/core/src/java/org/apache/lucene/store/NRTCachingDirectory.java:
##########
@@ -73,6 +73,7 @@ public class NRTCachingDirectory extends FilterDirectory
implements Accountable
new SingleInstanceLockFactory(),
ByteBuffersDataOutput::new,
(fileName, content) -> {
+ if (!isCachedFile(fileName)) return null;
Review Comment:
Could we split this onto three lines and add `{` ... `}` around the body?
And replace the `!` with `== false` to reduce chance of future accidental
refactoring bugs?
##########
lucene/core/src/java/org/apache/lucene/store/NRTCachingDirectory.java:
##########
@@ -73,6 +73,7 @@ public class NRTCachingDirectory extends FilterDirectory
implements Accountable
new SingleInstanceLockFactory(),
ByteBuffersDataOutput::new,
(fileName, content) -> {
+ if (!isCachedFile(fileName)) return null;
Review Comment:
Hmm, was this a separate bug? We were including this file's size in
`cacheSize` even if it is not cached?
Though, why would this lambda be called on a `fileName` that is not going to
be cached? I though the `if` in `NRTCachingDirectory.createOutput` would only
ask the `cacheDirectory` to create the output if the `fileName` was to be
cached?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
