sgup432 commented on code in PR #14412:
URL: https://github.com/apache/lucene/pull/14412#discussion_r2017693759
##########
lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java:
##########
@@ -122,12 +123,30 @@ public LRUQueryCache(
long maxRamBytesUsed,
Predicate<LeafReaderContext> leavesToCache,
float skipCacheFactor) {
+ this(maxSize, maxRamBytesUsed, leavesToCache, new
AtomicReference<>(skipCacheFactor));
+ }
+
+ /**
+ * Additionally, allows the ability to pass skipCacheFactor in form of
AtomicReference where the
+ * caller can dynamically update(in a thread safe way) its value by calling
skipCacheFactor.set()
+ * on their end.
+ */
+ public LRUQueryCache(
+ int maxSize,
+ long maxRamBytesUsed,
+ Predicate<LeafReaderContext> leavesToCache,
+ AtomicReference<Float> skipCacheFactor) {
Review Comment:
I actually made it a AtomicReference so that a caller can create that on
their end and pass this reference here. Via which the caller can change its
value dynamically.
Other way to set skipFactor dynamically would have been getter/setter like
you mentioned below. But then one cannot use an interface `QueryCache` to
initialize `LRUQueryCache`, and they are forced to use concrete
implementation(to get/set) which I think is not right.
```
QueryCache lruCache = new LRUQueryCache();
// Cannot accesss getter/setter until unless I typecast or use concrete
implementation directly.
```
Let me know what you think.
##########
lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java:
##########
@@ -122,12 +123,30 @@ public LRUQueryCache(
long maxRamBytesUsed,
Predicate<LeafReaderContext> leavesToCache,
float skipCacheFactor) {
+ this(maxSize, maxRamBytesUsed, leavesToCache, new
AtomicReference<>(skipCacheFactor));
+ }
+
+ /**
+ * Additionally, allows the ability to pass skipCacheFactor in form of
AtomicReference where the
+ * caller can dynamically update(in a thread safe way) its value by calling
skipCacheFactor.set()
+ * on their end.
+ */
+ public LRUQueryCache(
+ int maxSize,
+ long maxRamBytesUsed,
+ Predicate<LeafReaderContext> leavesToCache,
+ AtomicReference<Float> skipCacheFactor) {
Review Comment:
I actually made it a AtomicReference so that a caller can create that on
their end and pass this reference here. Via which the caller can change its
value dynamically.
Other way to set skipFactor dynamically would have been via getter/setter
like you mentioned below. But then one cannot use an interface `QueryCache` to
initialize `LRUQueryCache`, and they are forced to use concrete
implementation(to get/set) which I think is not right.
```
QueryCache lruCache = new LRUQueryCache();
// Cannot accesss getter/setter until unless I typecast or use concrete
implementation directly.
```
Let me know what you think.
--
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: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
