[jira] [Commented] (SOLR-9658) Caches should have an optional way to clean if idle for 'x' mins
[
https://issues.apache.org/jira/browse/SOLR-9658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16926186#comment-16926186
]
Hoss Man commented on SOLR-9658:
{quote}refactored cache impls to allow inserting synthetic entries, and changed
the unit tests to use these methods. It turned out that the management of
oldestEntry needs to be improved in all caches when we allow the creation time
in more recently added entries to go back...
{quote}
Ah interesting ... IIUC the existing code (in ConcurrentLFUCache for example)
just tracks "lastAccessed" for each cache entry (and "oldest" for the cache as
a whole) via an incremented counter across all entries – but now you're using
actual NANO_SECOND timestamps. This seems like an "ok" change (the API has
never exposed these "lastAccessed" values correct?) but I just want to double
check since you've looked at this & thought about it more then me: do you see
any risk here? (ie: please don't let me talk you into an Impl change that's "a
bad idea" just because it makes the kind of test I was advocating easier to
write)
Feedback on other aspects of the patch (all minor and/or nitpicks – in
generally this all seems solid) ...
* AFAICT there should no longer be any need to modify TimeSource /
TestTimeSource since tests no longer use/need advanceMs, correct ?
* {{SolrCache.MAX_IDLE_TIME}} doesn't seem to have a name consistent w/the
other variables in that interface ... seems like it should be
{{SolrCache.MAX_IDLE_TIME_PARAM}} ?
** There are also a couple of places in LFUCache and LRUCache (where other
existing {{*_PARAM}} constants are used) that seem to use the string literal
{{"maxIdleTime"}} instead of using that new variable.
* IIUC This isn't a mistake, it's a deliberate "clean up" change because the
existing code includes this {{put(RAM_BYTES_USED_PARAM, ...)}} twice a few
lines apart, correct? ...
{code:java}
-map.put(RAM_BYTES_USED_PARAM, ramBytesUsed());
+map.put("cumulative_idleEvictions", cidleEvictions);
{code}
* Is there any reason not to make these final in both ConcurrentLFUCache &
ConcurrentLRUCache?
{code:java}
private TimeSource timeSource = TimeSource.NANO_TIME;
private AtomicLong oldestEntry = new AtomicLong(0L);
{code}
* re: this line in {{TestLFUCache.testMaxIdleTimeEviction}} ...
** {{assertEquals("markAndSweep spurious run", 1, sweepFinished.getCount());}}
** a more thread safe way to have this type of assertion...
{code:java}
final AtomicLong numSweepsStarted = new AtomicLong(0); // NEW
final CountDownLatch sweepFinished = new CountDownLatch(1);
ConcurrentLRUCache cache = new ConcurrentLRUCache<>(6, 5,
5, 6, false, false, null, IDLE_TIME_SEC) {
@Override
public void markAndSweep() {
numSweepsStarted.incrementAndGet(); // NEW
super.markAndSweep();
sweepFinished.countDown();
}
};
...
assertEquals("markAndSweep spurious runs", 0L, numSweepsStarted.get()); //
CHANGED
{code}
** I think that pattern exists in another test as well?
* we need to make sure the javadocs & ref-guide are updated to cover this new
option, and be clear to users on how it interacts with other things (ie: that
the idle sweep happens before the other sweeps and trumps things like the
"entry size" checks)
> Caches should have an optional way to clean if idle for 'x' mins
>
>
> Key: SOLR-9658
> URL: https://issues.apache.org/jira/browse/SOLR-9658
> Project: Solr
> Issue Type: New Feature
>Reporter: Noble Paul
>Assignee: Andrzej Bialecki
>Priority: Major
> Fix For: 8.3
>
> Attachments: SOLR-9658.patch, SOLR-9658.patch, SOLR-9658.patch,
> SOLR-9658.patch, SOLR-9658.patch, SOLR-9658.patch
>
>
> If a cache is idle for long, it consumes precious memory. It should be
> configurable to clear the cache if it was not accessed for 'x' secs. The
> cache configuration can have an extra config {{maxIdleTime}} . if we wish it
> to the cleaned after 10 mins of inactivity set it to {{maxIdleTime=600}}.
> [~dragonsinth] would it be a solution for the memory leak you mentioned?
--
This message was sent by Atlassian Jira
(v8.3.2#803003)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-9658) Caches should have an optional way to clean if idle for 'x' mins
[
https://issues.apache.org/jira/browse/SOLR-9658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16922432#comment-16922432
]
David Smiley commented on SOLR-9658:
Sure; I don't mean to stand in your way.
> Caches should have an optional way to clean if idle for 'x' mins
>
>
> Key: SOLR-9658
> URL: https://issues.apache.org/jira/browse/SOLR-9658
> Project: Solr
> Issue Type: New Feature
>Reporter: Noble Paul
>Assignee: Andrzej Bialecki
>Priority: Major
> Fix For: 8.3
>
> Attachments: SOLR-9658.patch, SOLR-9658.patch, SOLR-9658.patch,
> SOLR-9658.patch, SOLR-9658.patch, SOLR-9658.patch
>
>
> If a cache is idle for long, it consumes precious memory. It should be
> configurable to clear the cache if it was not accessed for 'x' secs. The
> cache configuration can have an extra config {{maxIdleTime}} . if we wish it
> to the cleaned after 10 mins of inactivity set it to {{maxIdleTime=600}}.
> [~dragonsinth] would it be a solution for the memory leak you mentioned?
--
This message was sent by Atlassian Jira
(v8.3.2#803003)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-9658) Caches should have an optional way to clean if idle for 'x' mins
[
https://issues.apache.org/jira/browse/SOLR-9658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16922269#comment-16922269
]
Andrzej Bialecki commented on SOLR-9658:
-
[~dsmiley] thanks, I wasn't aware of that Jira. This makes sense - I'll look
into that issue.
Still, even if we eventually replace our cache implementations with Caffeine
we're probably going to do this in 9.0 and not in 8.x, so the improvement
proposed in this issue is still applicable.
> Caches should have an optional way to clean if idle for 'x' mins
>
>
> Key: SOLR-9658
> URL: https://issues.apache.org/jira/browse/SOLR-9658
> Project: Solr
> Issue Type: New Feature
>Reporter: Noble Paul
>Assignee: Andrzej Bialecki
>Priority: Major
> Fix For: 8.3
>
> Attachments: SOLR-9658.patch, SOLR-9658.patch, SOLR-9658.patch,
> SOLR-9658.patch, SOLR-9658.patch, SOLR-9658.patch
>
>
> If a cache is idle for long, it consumes precious memory. It should be
> configurable to clear the cache if it was not accessed for 'x' secs. The
> cache configuration can have an extra config {{maxIdleTime}} . if we wish it
> to the cleaned after 10 mins of inactivity set it to {{maxIdleTime=600}}.
> [~dragonsinth] would it be a solution for the memory leak you mentioned?
--
This message was sent by Atlassian Jira
(v8.3.2#803003)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-9658) Caches should have an optional way to clean if idle for 'x' mins
[
https://issues.apache.org/jira/browse/SOLR-9658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16921723#comment-16921723
]
David Smiley commented on SOLR-9658:
Do we as a project really want to implement our own caches or is it about time
that we use other popular caches like
[Caffeine|https://github.com/ben-manes/caffeine]? Don't get me wrong, I love
writing code, and working on caches is fun, but I'd rather we use one of our
many already existing dependencies for this common task. SOLR-8241 is about
adding Caffeine to Solr.
> Caches should have an optional way to clean if idle for 'x' mins
>
>
> Key: SOLR-9658
> URL: https://issues.apache.org/jira/browse/SOLR-9658
> Project: Solr
> Issue Type: New Feature
>Reporter: Noble Paul
>Assignee: Andrzej Bialecki
>Priority: Major
> Fix For: 8.3
>
> Attachments: SOLR-9658.patch, SOLR-9658.patch, SOLR-9658.patch,
> SOLR-9658.patch, SOLR-9658.patch, SOLR-9658.patch
>
>
> If a cache is idle for long, it consumes precious memory. It should be
> configurable to clear the cache if it was not accessed for 'x' secs. The
> cache configuration can have an extra config {{maxIdleTime}} . if we wish it
> to the cleaned after 10 mins of inactivity set it to {{maxIdleTime=600}}.
> [~dragonsinth] would it be a solution for the memory leak you mentioned?
--
This message was sent by Atlassian Jira
(v8.3.2#803003)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-9658) Caches should have an optional way to clean if idle for 'x' mins
[
https://issues.apache.org/jira/browse/SOLR-9658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16915748#comment-16915748
]
Andrzej Bialecki commented on SOLR-9658:
-
Updated patch synced with the current master. [~hossman] if this looks ok I'd
like to commit it shortly.
> Caches should have an optional way to clean if idle for 'x' mins
>
>
> Key: SOLR-9658
> URL: https://issues.apache.org/jira/browse/SOLR-9658
> Project: Solr
> Issue Type: New Feature
>Reporter: Noble Paul
>Assignee: Andrzej Bialecki
>Priority: Major
> Fix For: 8.3
>
> Attachments: SOLR-9658.patch, SOLR-9658.patch, SOLR-9658.patch,
> SOLR-9658.patch, SOLR-9658.patch, SOLR-9658.patch
>
>
> If a cache is idle for long, it consumes precious memory. It should be
> configurable to clear the cache if it was not accessed for 'x' secs. The
> cache configuration can have an extra config {{maxIdleTime}} . if we wish it
> to the cleaned after 10 mins of inactivity set it to {{maxIdleTime=600}}.
> [~dragonsinth] would it be a solution for the memory leak you mentioned?
--
This message was sent by Atlassian Jira
(v8.3.2#803003)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-9658) Caches should have an optional way to clean if idle for 'x' mins
[
https://issues.apache.org/jira/browse/SOLR-9658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16905553#comment-16905553
]
Andrzej Bialecki commented on SOLR-9658:
-
Here's an updated patch - I tried to address all your concerns:
* removed the CacheListener API
* refactored cache impls to allow inserting synthetic entries, and changed the
unit tests to use these methods. It turned out that the management of
{{oldestEntry}} needs to be improved in all caches when we allow the creation
time in more recently added entries to go back...
* currently unit tests test only the first part - "what happens when
markAndSweep() is called on a cache with specific entries". I'll add the other
part if this patch looks ok.
> Caches should have an optional way to clean if idle for 'x' mins
>
>
> Key: SOLR-9658
> URL: https://issues.apache.org/jira/browse/SOLR-9658
> Project: Solr
> Issue Type: New Feature
>Reporter: Noble Paul
>Assignee: Andrzej Bialecki
>Priority: Major
> Fix For: 8.3
>
> Attachments: SOLR-9658.patch, SOLR-9658.patch, SOLR-9658.patch,
> SOLR-9658.patch, SOLR-9658.patch
>
>
> If a cache is idle for long, it consumes precious memory. It should be
> configurable to clear the cache if it was not accessed for 'x' secs. The
> cache configuration can have an extra config {{maxIdleTime}} . if we wish it
> to the cleaned after 10 mins of inactivity set it to {{maxIdleTime=600}}.
> [~dragonsinth] would it be a solution for the memory leak you mentioned?
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-9658) Caches should have an optional way to clean if idle for 'x' mins
[
https://issues.apache.org/jira/browse/SOLR-9658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16904200#comment-16904200
]
Hoss Man commented on SOLR-9658:
* i should have noticed/mentioned this in the last patch but: any method
(including your new {{markAndSweepByIdleTime()}} that that expects to be called
only when markAndSweepLock is already held should really start with {{assert
markAndSweepLock.isHeldByCurrentThread();}}
* this patch still seems to modify TestJavaBinCodec unneccessarily? (now that
you re-added the backcompat constructor)
* i don't really think it's a good idea to add these {{CacheListener}} /
{{EvictionListener}} APIs at this point w/o a lot more consideration of their
lifecycle / usage
** I know you introduced them in response to my suggestion to add hooks for
monitoring in tests, but they don't _currently_ seem more useful in the tests
then some of the specific suggestions i made before (more comments on this
below) and the APIs don't seem to be thought through enough to be generally
useful later w/o a lot of re-working...
*** Examples: if the point of creating {{CacheListener}} now is to be able to
add more methods/hooks to it later, then is why is only {{EvictionListener}}
passed down to the {{ConcurrentXXXCache}} impls instead of the entire
{{CacheListener}} ?
*** And why are there 2 distinct {{EvictionListener}} interfaces, instead of
just a common one?
** ... so it would probably be safer/cleaner to avoid adding these APIs now
since there are simpler alternatives available for the tests?
* Re: "...plus adding support for artificially "advancing" the time" ... this
seems overly complex?
** None of the suggestions i made for improving the reliability/coverage of
the test require needing to fake the "now" clock: just being able to insert
synthetic entries into the cache with artifically old timestamps – which could
be done by refactoring out the middle of {{put(...)}} into a new
{{putCacheEntry(CacheEntry ... )}} method that would let the (test) caller set
an arbitrary {{lastAccessed}} value...
{code:java}
/**
* Useable by tests to create synthetic cache entries, also called by {@link
#put}
* @lucene.internal
*/
public CacheEntry putCacheEntry(CacheEntry e) {
CacheEntry oldCacheEntry = map.put(key, e);
int currentSize;
if (oldCacheEntry == null) {
currentSize = stats.size.incrementAndGet();
ramBytes.addAndGet(e.ramBytesUsed() + HASHTABLE_RAM_BYTES_PER_ENTRY); //
added key + value + entry
} else {
currentSize = stats.size.get();
ramBytes.addAndGet(-oldCacheEntry.ramBytesUsed());
ramBytes.addAndGet(e.ramBytesUsed());
}
if (islive) {
stats.putCounter.increment();
} else {
stats.nonLivePutCounter.increment();
}
return oldCacheEntry;
}
{code}
** ...that way tests could "setup" a cache containing arbitrary entries (of
arbitrary size, with arbitrary create/access times that could be from weeks in
the past) and then very precisely inspect the results of the cache after
calling {{markAndSweep()}}
*** or some other new {{triggerCleanupIfNeeded()}} method that can encapsualte
all of the existing {{// Check if we need to clear out old entries from the
cache ...}} logic currently at the end of {{put()}}
* In general, i really think testing of functionality like this should really
focus on testing "what exactly happens when markAndSeep() is called on a cache
containing a very specific set of values?" indepdnent from "does markAndSweep()
get called eventually & automatically if i configure maxIdleTime?"
** the former can be tested w/o the need of any cleanup threads or faking the
TimeSource
** the later can be tested w/o the need of a {{CacheListener}} or
{{EvictionListener}} API (or a fake TimeSource) – just create an anonymous
subclass of {{ConcurrentXXXCache}} who'se markAndSweep() method decrements a
CountDownLatch tha the test tread is waiting on
** isolating the testing of these different concepts not only makes it easier
to test more complex aspects of how {{markAndSweep()}} is expected to work (ie:
"assert exactly which entries are removed if the sum of the sizes == X ==
(ramUpperWatermark + Y) but the two smallest entries (whose total size = Y + 1)
are the only one with an accessTime older then the idleTime") but also makes it
easier to understand & debug failures down the road -if- _when_ they happen.
*** as things stand in your patch, -if- _when_ the "did not evict entries in
time" assert (eventually) trips in a future jenkins build, we won't immediately
be able to tell (w/o added logging) if that's because of a bug in the
{{CleanupThread}} that prevented if from calling {{markAndSweep();}} or a bug
in {{SimTimeSource.advanceMs()}} ; or a bug somewhere in the cache that
prevented {{markAndSweep()}} from recognizing those entries were old; or just a
heavily loaded VM CPU
[jira] [Commented] (SOLR-9658) Caches should have an optional way to clean if idle for 'x' mins
[
https://issues.apache.org/jira/browse/SOLR-9658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16903103#comment-16903103
]
Andrzej Bialecki commented on SOLR-9658:
-
Thanks Hoss for the detailed review! I updated the patch to include the changes
after your review:
* fixed ConcurrentLRUCache to sweep by idle time first.
* fixed CleanupThread to not need the additional arg.
* fixed the constructors to preserve back-compat, and fixed
TemplateUpdateProcessorFactory to use original defaults (no maxIdleTime).
* fixed unit tests to not rely on Thread.sleep - this required a bit more
changes in order to expose the eviction listener in a uniform way across cache
impls, plus adding support for artificially "advancing" the time. The new
{{CacheListener}} callback allows us to add more instrumentation later, eg. to
monitor the sweeps by type and collect pre/post sweep stats, etc.
> Caches should have an optional way to clean if idle for 'x' mins
>
>
> Key: SOLR-9658
> URL: https://issues.apache.org/jira/browse/SOLR-9658
> Project: Solr
> Issue Type: New Feature
>Reporter: Noble Paul
>Assignee: Andrzej Bialecki
>Priority: Major
> Fix For: 8.3
>
> Attachments: SOLR-9658.patch, SOLR-9658.patch, SOLR-9658.patch,
> SOLR-9658.patch
>
>
> If a cache is idle for long, it consumes precious memory. It should be
> configurable to clear the cache if it was not accessed for 'x' secs. The
> cache configuration can have an extra config {{maxIdleTime}} . if we wish it
> to the cleaned after 10 mins of inactivity set it to {{maxIdleTime=600}}.
> [~dragonsinth] would it be a solution for the memory leak you mentioned?
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-9658) Caches should have an optional way to clean if idle for 'x' mins
[
https://issues.apache.org/jira/browse/SOLR-9658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16902561#comment-16902561
]
Hoss Man commented on SOLR-9658:
* i don't see anything that updates {{oldestEntryNs}} except
{{markAndSweepByIdleTime}} ?
** this means that {{markAndSweep()}} may unneccessarily call
{{markAndSweepByIdleTime()}} (looping over every entry) even if everything
older then the maxIdleTime has already been purged by earlier method calls like
{{markAndSweepByCacheSize()}} or {{markAndSweepByRamSize()}}
** off the top of my head, i can't think of an efficient way to "update"
{{oldestEntryNs}} in some place like {{postRemoveEntry()}} w/o scanning every
cache entry again, but...
** why not move {{markAndSweepByIdleTime()}} _before_
{{markAndSweepByCacheSize()}} and {{markAndSweepByRamSize()}} ?
*** since the {{postRemoveEntry()}} calls made as a result of any eviction due
to idle time *can* (and already do) efficiently update the results of
{{size()}} and {{ramBytesUsed()}} that could potentially save the need for
those additional scans of the cache in many situations.
* rather then complicating the patch by changing the constructor of the
{{CleanupThread}} class(es) to take in the maxIdle values directly, why not
read that info from a (new) method on the ConcurrentXXXCache objects already
passed to the constructors?
** with some small tweaks to the while loop, the {{wait()}} call could actual
read this value dynamically from the cache element, eliminating the need to
call {{setRunCleanupThread()}} from inside {{setMaxIdleTime()}} in the event
that the value is changed dynamically.
*** which is currently broken anyway since {{setRunCleanupThread()}} is
currently a No-Op if {{this.runCleanupThread}} is true and {{cleanupThread}} is
already non-null.
** assuming {{CleanupThread}} is changed to dynamically read the maxIdleTime
directly from the cache, {{setMaxIdleTime()}} could just call {{wakeThread()}}
if the new maxIdleTime is less then the old maxIdleTime
*** or leave the call to {{setRunCleanupThread()}} as is, but change the {{if
(cleanupThread == null)}} condition of {{setRunCleanupThread()}} to have an
"else" code path that calls {{wakeThread()}} so it will call {{markAndSweep()}}
(with the udpated settings) and then re-wait (with the new maxIdleTime)
* although not likely to be problematic in practice, you've broken backcompat
on the public "ConcurrentXXXCache" class(es) by adding an arg to the
constructor.
** i would suggest adding a new constructor instead, and making the old one
call the new one with "-1" – if for no other reason then to simplify the touch
points / discussion in the patch...
** ie: in order to make this change, you had to modify both
{{TestJavaBinCodec}} and {{TemplateUpdateProcessorFactory}} – but you wound up
not using a backcompat equivilent value in {{TemplateUpdateProcessorFactory}}
so your changes actually modify the behavior of that (end user facing class) in
an undocumented way (that users can't override, and may actually have some
noticible performance impacts on "put" since that existing usage doens't
involve the cleanup thread) which should be discussed before committing (but
are largely unrelated to the goals in this jira)
* under no circumstances should we be committing new test code that makes
arbitrary {{Thread.sleep(5000)}} calls
** i am willing to say categorically that this approach: DOES. NOT. WORK. –
and has represented an overwelming percentage of the root causes of our tests
being unreliable
*** there is no garuntee the JVM will sleep as long as you ask it too
(particularly on virtual hardware)
*** there is no garuntee that "background threads/logic" will be
scheduled/finished during the "sleep"
** it is far better to add whatever {{@lucene.internal}} methods we need to
"hook into" the core code from test code and have white-box / grey-box tests
that ensure methods get called when we expect, ex:
*** if we want to test that the user level configuration results in the
appropriate values being set on the underlying objects, we should add public
getter methods for those values to those classes, and have the test reach into
the SolrCore to get those objects and assert the expected results on those
methods (NOT just "wait" to see the code run and have the expected side effects)
*** if we want to test that {{ConcurrentXXXCache.markAndSweep()}} gets called
by the {{CleanupThread}} _eventually_ when maxIdle time is configured even if
nothing calls {{wakeThread()}} then we should use a mock/subclass of the
ConcurrentXXXCache that overrides {{markAndSweep()}} to set a latch that we can
{{await(...)}} on from the test code.
*** if we want to test that calls to {{ConcurrentXXXCache.markAndSweep()}}
result in items being removed if their {{createTime}} is "too old" then we
should add a special internal only version of
[jira] [Commented] (SOLR-9658) Caches should have an optional way to clean if idle for 'x' mins
[
https://issues.apache.org/jira/browse/SOLR-9658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16901430#comment-16901430
]
Andrzej Bialecki commented on SOLR-9658:
-
The latest patch adds also support for {{maxIdleTime}} to {{FastLRUCache}}.
> Caches should have an optional way to clean if idle for 'x' mins
>
>
> Key: SOLR-9658
> URL: https://issues.apache.org/jira/browse/SOLR-9658
> Project: Solr
> Issue Type: New Feature
>Reporter: Noble Paul
>Assignee: Andrzej Bialecki
>Priority: Major
> Attachments: SOLR-9658.patch, SOLR-9658.patch
>
>
> If a cache is idle for long, it consumes precious memory. It should be
> configurable to clear the cache if it was not accessed for 'x' secs. The
> cache configuration can have an extra config {{maxIdleTime}} . if we wish it
> to the cleaned after 10 mins of inactivity set it to {{maxIdleTime=600}}.
> [~dragonsinth] would it be a solution for the memory leak you mentioned?
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-9658) Caches should have an optional way to clean if idle for 'x' mins
[
https://issues.apache.org/jira/browse/SOLR-9658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16901327#comment-16901327
]
Andrzej Bialecki commented on SOLR-9658:
-
This patch implements the following:
* adds {{maxIdleTime}} config param to {{LFUCache}} and {{LRUCache}}.
* {{FastLRUCache}} is in the works but the eviction algorithm there is quite
complicated and I'm not sure I fully understand it... Having said that, maybe
we should do an additional full sweep and remove expired entries regardless of
the existing algorithm.
* entries are expired on {{put}}. If a cleanup thread is used in {{LFUCache}}
then it also wakes up every {{maxIdleTime}} to expire old entries even if
there's no {{put}}.
* cache entries are now marked with epoch time (ns) instead of the generation
counter. This allows us to evict entries based on real elapsed time, and using
epoch time makes debugging somewhat easier at no additional cost compared to
nano time.
* unit tests
> Caches should have an optional way to clean if idle for 'x' mins
>
>
> Key: SOLR-9658
> URL: https://issues.apache.org/jira/browse/SOLR-9658
> Project: Solr
> Issue Type: New Feature
>Reporter: Noble Paul
>Assignee: Andrzej Bialecki
>Priority: Major
> Attachments: SOLR-9658.patch
>
>
> If a cache is idle for long, it consumes precious memory. It should be
> configurable to clear the cache if it was not accessed for 'x' secs. The
> cache configuration can have an extra config {{maxIdleTime}} . if we wish it
> to the cleaned after 10 mins of inactivity set it to {{maxIdleTime=600}}.
> [~dragonsinth] would it be a solution for the memory leak you mentioned?
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-9658) Caches should have an optional way to clean if idle for 'x' mins
[
https://issues.apache.org/jira/browse/SOLR-9658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16898382#comment-16898382
]
Erick Erickson commented on SOLR-9658:
--
And good luck figuring out something that'll satisfy _everybody_ ;)
> Caches should have an optional way to clean if idle for 'x' mins
>
>
> Key: SOLR-9658
> URL: https://issues.apache.org/jira/browse/SOLR-9658
> Project: Solr
> Issue Type: New Feature
>Reporter: Noble Paul
>Assignee: Andrzej Bialecki
>Priority: Major
>
> If a cache is idle for long, it consumes precious memory. It should be
> configurable to clear the cache if it was not accessed for 'x' secs. The
> cache configuration can have an extra config {{maxIdleTime}} . if we wish it
> to the cleaned after 10 mins of inactivity set it to {{maxIdleTime=600}}.
> [~dragonsinth] would it be a solution for the memory leak you mentioned?
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-9658) Caches should have an optional way to clean if idle for 'x' mins
[
https://issues.apache.org/jira/browse/SOLR-9658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16898314#comment-16898314
]
Andrzej Bialecki commented on SOLR-9658:
-
There's a trade-off between the cache size and hit ratio - we would like to use
as much memory as possible to get a good hit ratio, but this is costly so we
also want to use as little memory as possible while retaining a good hit ratio
;)
So there's a certain sweet-spot for dynamically adjusting cache size, where we
use just enough memory to get a good hit ratio. If hit ratio falls below a
threshold we can reduce the memory (because this means that many entries are
useless), if hit ratio is high then we can increase the memory because the
cache is full.
> Caches should have an optional way to clean if idle for 'x' mins
>
>
> Key: SOLR-9658
> URL: https://issues.apache.org/jira/browse/SOLR-9658
> Project: Solr
> Issue Type: New Feature
>Reporter: Noble Paul
>Assignee: Andrzej Bialecki
>Priority: Major
>
> If a cache is idle for long, it consumes precious memory. It should be
> configurable to clear the cache if it was not accessed for 'x' secs. The
> cache configuration can have an extra config {{maxIdleTime}} . if we wish it
> to the cleaned after 10 mins of inactivity set it to {{maxIdleTime=600}}.
> [~dragonsinth] would it be a solution for the memory leak you mentioned?
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-9658) Caches should have an optional way to clean if idle for 'x' mins
[
https://issues.apache.org/jira/browse/SOLR-9658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16897182#comment-16897182
]
Erick Erickson commented on SOLR-9658:
--
Moving over here from 13668:
I'm a little iffy on this. One advantage with filling up the caches to the
limit then keeping it at the limit is that unreasonable limits are more like to
be found during testing. Here's the scenario:
I set up filterCache with a size of 1,000,000
I test for a while with something like fq=timestamp:[NOW-1DAY TO NOW]
I'm indexing a lot and bouncing the server so the cache never grows too large
I put it in production on an unchanging index and generate OOMs.
I've seen this case "in the wild" so it's not entirely theoretical.
Similar situations could occur with aging out entries, it works fine until the
system comes under heavy load where all the cache entries are relatively young.
That said, reducing the memory pressure is always a good thing. Perhaps where
we eventually end up is with some heuristics around dynamically resizing caches
based on memory pressure or aging out cache entries more quickly based on
memory pressure etc., in which case my worries evaporate.
> Caches should have an optional way to clean if idle for 'x' mins
>
>
> Key: SOLR-9658
> URL: https://issues.apache.org/jira/browse/SOLR-9658
> Project: Solr
> Issue Type: New Feature
>Reporter: Noble Paul
>Assignee: Andrzej Bialecki
>Priority: Major
>
> If a cache is idle for long, it consumes precious memory. It should be
> configurable to clear the cache if it was not accessed for 'x' secs. The
> cache configuration can have an extra config {{maxIdleTime}} . if we wish it
> to the cleaned after 10 mins of inactivity set it to {{maxIdleTime=600}}.
> [~dragonsinth] would it be a solution for the memory leak you mentioned?
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-9658) Caches should have an optional way to clean if idle for 'x' mins
[
https://issues.apache.org/jira/browse/SOLR-9658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15586057#comment-15586057
]
Noble Paul commented on SOLR-9658:
--
[~dragonsinth] I can't think of why reopening a searcher wouldn't clear caches.
if we can isolate that into a testcase it will be great
> Caches should have an optional way to clean if idle for 'x' mins
>
>
> Key: SOLR-9658
> URL: https://issues.apache.org/jira/browse/SOLR-9658
> Project: Solr
> Issue Type: New Feature
> Security Level: Public(Default Security Level. Issues are Public)
>Reporter: Noble Paul
>
> If a cache is idle for long, it consumes precious memory. It should be
> configurable to clear the cache if it was not accessed for 'x' secs. The
> cache configuration can have an extra config {{maxIdleTime}} . if we wish it
> to the cleaned after 10 mins of inactivity set it to {{maxIdleTime=600}}.
> [~dragonsinth] would it be a solution for the memory leak you mentioned?
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-9658) Caches should have an optional way to clean if idle for 'x' mins
[
https://issues.apache.org/jira/browse/SOLR-9658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15586028#comment-15586028
]
Scott Blum commented on SOLR-9658:
--
That would be awesome, I believe that would work.
Related question: do you have insight into why doesn't auto soft commit
wouldn't be clearing caches already? Assuming I don't want auto warmed
queries, I might naively think that an auto soft commit should have the effect
of evicting caches since it would invalidate the results. Do you know why that
doesn't seem to happen?
> Caches should have an optional way to clean if idle for 'x' mins
>
>
> Key: SOLR-9658
> URL: https://issues.apache.org/jira/browse/SOLR-9658
> Project: Solr
> Issue Type: New Feature
> Security Level: Public(Default Security Level. Issues are Public)
>Reporter: Noble Paul
>
> If a cache is idle for long, it consumes precious memory. It should be
> configurable to clear the cache if it was not accessed for 'x' secs. The
> cache configuration can have an extra config {{maxIdleTime}} . if we wish it
> to the cleaned after 10 mins of inactivity set it to {{maxIdleTime=600}}.
> [~dragonsinth] would it be a solution for the memory leak you mentioned?
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
