[jira] [Commented] (LUCENE-7410) Make cache keys and closed listeners less trappy
[
https://issues.apache.org/jira/browse/LUCENE-7410?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15894691#comment-15894691
]
ASF subversion and git services commented on LUCENE-7410:
-
Commit fbc844d33439efc1c5c6fee5547715d1a1b0db83 in lucene-solr's branch
refs/heads/master from [~jpountz]
[ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=fbc844d ]
LUCENE-7410: Fix test bug.
> Make cache keys and closed listeners less trappy
>
>
> Key: LUCENE-7410
> URL: https://issues.apache.org/jira/browse/LUCENE-7410
> Project: Lucene - Core
> Issue Type: Bug
>Reporter: Adrien Grand
> Fix For: master (7.0)
>
> Attachments: LUCENE-7410.patch, LUCENE-7410.patch, LUCENE-7410.patch
>
>
> IndexReader currently exposes getCoreCacheKey(),
> getCombinedCoreAndDeletesKey(), addCoreClosedListener() and
> addReaderClosedListener(). They are typically used to manage resources whose
> lifetime needs to mimic the lifetime of segments/indexes, typically caches.
> I think this is trappy for various reasons:
> h3. Memory leaks
> When maintaining a cache, entries are added to the cache based on the cache
> key and then evicted using the cache key that is given back by the close
> listener, so it is very important that both keys are the same.
> But if a filter reader happens to delegate get*Key() and not
> add*ClosedListener() or vice-versa then there is potential for a memory leak
> since the closed listener will be called on a different key and entries will
> never be evicted from the cache.
> h3. Lifetime expectations
> The expectation of using the core cache key is that it will not change in
> case of deletions, but this is only true on SegmentReader and LeafReader
> impls that delegate to it. Other implementations such as composite readers or
> parallel leaf readers use the same key for "core" and "combined core and
> deletes".
> h3. Throw-away wrappers cause cache trashing
> An application might want to either expose more (with a ParrallelReader or
> MultiReader) or less information (by filtering fields/docs that can be seen)
> depending on the user who is logged in. In that case the application would
> typically maintain a DirectoryReader and then wrap it per request depending
> on the logged user and throw away the wrapper once the request is completed.
> The problem is that these wrappers have their own cache keys and the
> application may build something costly and put it in a cache to throw it away
> a couple milliseconds later. I would rather like for such readers to have a
> way to opt out from caching on order to avoid this performance trap.
> h3. Type safety
> The keys that are exposed are plain java.lang.Object instances, which
> requires caches to look like a {{Map}} which makes it very easy to
> either try to get, put or remove on the wrong object since any object would
> be accepted.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-7410) Make cache keys and closed listeners less trappy
[
https://issues.apache.org/jira/browse/LUCENE-7410?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15894692#comment-15894692
]
Adrien Grand commented on LUCENE-7410:
--
Thanks Steve, it was a test bug, I just pushed a fix.
> Make cache keys and closed listeners less trappy
>
>
> Key: LUCENE-7410
> URL: https://issues.apache.org/jira/browse/LUCENE-7410
> Project: Lucene - Core
> Issue Type: Bug
>Reporter: Adrien Grand
> Fix For: master (7.0)
>
> Attachments: LUCENE-7410.patch, LUCENE-7410.patch, LUCENE-7410.patch
>
>
> IndexReader currently exposes getCoreCacheKey(),
> getCombinedCoreAndDeletesKey(), addCoreClosedListener() and
> addReaderClosedListener(). They are typically used to manage resources whose
> lifetime needs to mimic the lifetime of segments/indexes, typically caches.
> I think this is trappy for various reasons:
> h3. Memory leaks
> When maintaining a cache, entries are added to the cache based on the cache
> key and then evicted using the cache key that is given back by the close
> listener, so it is very important that both keys are the same.
> But if a filter reader happens to delegate get*Key() and not
> add*ClosedListener() or vice-versa then there is potential for a memory leak
> since the closed listener will be called on a different key and entries will
> never be evicted from the cache.
> h3. Lifetime expectations
> The expectation of using the core cache key is that it will not change in
> case of deletions, but this is only true on SegmentReader and LeafReader
> impls that delegate to it. Other implementations such as composite readers or
> parallel leaf readers use the same key for "core" and "combined core and
> deletes".
> h3. Throw-away wrappers cause cache trashing
> An application might want to either expose more (with a ParrallelReader or
> MultiReader) or less information (by filtering fields/docs that can be seen)
> depending on the user who is logged in. In that case the application would
> typically maintain a DirectoryReader and then wrap it per request depending
> on the logged user and throw away the wrapper once the request is completed.
> The problem is that these wrappers have their own cache keys and the
> application may build something costly and put it in a cache to throw it away
> a couple milliseconds later. I would rather like for such readers to have a
> way to opt out from caching on order to avoid this performance trap.
> h3. Type safety
> The keys that are exposed are plain java.lang.Object instances, which
> requires caches to look like a {{Map}} which makes it very easy to
> either try to get, put or remove on the wrong object since any object would
> be accepted.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-7410) Make cache keys and closed listeners less trappy
[
https://issues.apache.org/jira/browse/LUCENE-7410?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15894320#comment-15894320
]
Steve Rowe commented on LUCENE-7410:
Policeman Jenkins found a reproducing seed for a test method introduced on this
issue [https://jenkins.thetaphi.de/job/Lucene-Solr-master-Linux/19092/] (at
https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=df6f830 - seed
reproduces with this hash):
{noformat}
[junit4] 2> NOTE: reproduce with: ant test
-Dtestcase=TestSlowCompositeReaderWrapper -Dtests.method=testOrdMapsAreCached
-Dtests.seed=ED06E16D2F7E2C9F -Dtests.multiplier=3 -Dtests.slow=true
-Dtests.locale=en-BS -Dtests.timezone=Asia/Kashgar -Dtests.asserts=true
-Dtests.file.encoding=UTF-8
[junit4] FAILURE 0.01s J2 |
TestSlowCompositeReaderWrapper.testOrdMapsAreCached <<<
[junit4]> Throwable #1: java.lang.AssertionError
[junit4]>at
__randomizedtesting.SeedInfo.seed([ED06E16D2F7E2C9F:B1C7CA871734CF6C]:0)
[junit4]>at
org.apache.solr.index.TestSlowCompositeReaderWrapper.testOrdMapsAreCached(TestSlowCompositeReaderWrapper.java:111)
[junit4]>at
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
[junit4]>at
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
[junit4]>at
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
[junit4]>at
java.base/java.lang.reflect.Method.invoke(Method.java:547)
[junit4]>at java.base/java.lang.Thread.run(Thread.java:844)
[junit4] 2> NOTE: test params are: codec=Asserting(Lucene70): {},
docValues:{sorted=DocValuesFormat(name=Lucene70),
sorted_set=DocValuesFormat(name=Lucene70)}, maxPointsInLeafNode=1289,
maxMBSortInHeap=5.014157490710387, sim=RandomSimilarity(queryNorm=true): {},
locale=en-BS, timezone=Asia/Kashgar
[junit4] 2> NOTE: Linux 4.4.0-53-generic amd64/Oracle Corporation 9-ea
(64-bit)/cpus=12,threads=1,free=204857864,total=518979584
{noformat}
> Make cache keys and closed listeners less trappy
>
>
> Key: LUCENE-7410
> URL: https://issues.apache.org/jira/browse/LUCENE-7410
> Project: Lucene - Core
> Issue Type: Bug
>Reporter: Adrien Grand
> Fix For: master (7.0)
>
> Attachments: LUCENE-7410.patch, LUCENE-7410.patch, LUCENE-7410.patch
>
>
> IndexReader currently exposes getCoreCacheKey(),
> getCombinedCoreAndDeletesKey(), addCoreClosedListener() and
> addReaderClosedListener(). They are typically used to manage resources whose
> lifetime needs to mimic the lifetime of segments/indexes, typically caches.
> I think this is trappy for various reasons:
> h3. Memory leaks
> When maintaining a cache, entries are added to the cache based on the cache
> key and then evicted using the cache key that is given back by the close
> listener, so it is very important that both keys are the same.
> But if a filter reader happens to delegate get*Key() and not
> add*ClosedListener() or vice-versa then there is potential for a memory leak
> since the closed listener will be called on a different key and entries will
> never be evicted from the cache.
> h3. Lifetime expectations
> The expectation of using the core cache key is that it will not change in
> case of deletions, but this is only true on SegmentReader and LeafReader
> impls that delegate to it. Other implementations such as composite readers or
> parallel leaf readers use the same key for "core" and "combined core and
> deletes".
> h3. Throw-away wrappers cause cache trashing
> An application might want to either expose more (with a ParrallelReader or
> MultiReader) or less information (by filtering fields/docs that can be seen)
> depending on the user who is logged in. In that case the application would
> typically maintain a DirectoryReader and then wrap it per request depending
> on the logged user and throw away the wrapper once the request is completed.
> The problem is that these wrappers have their own cache keys and the
> application may build something costly and put it in a cache to throw it away
> a couple milliseconds later. I would rather like for such readers to have a
> way to opt out from caching on order to avoid this performance trap.
> h3. Type safety
> The keys that are exposed are plain java.lang.Object instances, which
> requires caches to look like a {{Map}} which makes it very easy to
> either try to get, put or remove on the wrong object since any object would
> be accepted.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional
[jira] [Commented] (LUCENE-7410) Make cache keys and closed listeners less trappy
[
https://issues.apache.org/jira/browse/LUCENE-7410?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15890723#comment-15890723
]
Adrien Grand commented on LUCENE-7410:
--
It was a test bug introduced by this change, which happens when the created
IndexSearcher wraps a threadpool. I just pushed a fix.
> Make cache keys and closed listeners less trappy
>
>
> Key: LUCENE-7410
> URL: https://issues.apache.org/jira/browse/LUCENE-7410
> Project: Lucene - Core
> Issue Type: Bug
>Reporter: Adrien Grand
> Fix For: master (7.0)
>
> Attachments: LUCENE-7410.patch, LUCENE-7410.patch, LUCENE-7410.patch
>
>
> IndexReader currently exposes getCoreCacheKey(),
> getCombinedCoreAndDeletesKey(), addCoreClosedListener() and
> addReaderClosedListener(). They are typically used to manage resources whose
> lifetime needs to mimic the lifetime of segments/indexes, typically caches.
> I think this is trappy for various reasons:
> h3. Memory leaks
> When maintaining a cache, entries are added to the cache based on the cache
> key and then evicted using the cache key that is given back by the close
> listener, so it is very important that both keys are the same.
> But if a filter reader happens to delegate get*Key() and not
> add*ClosedListener() or vice-versa then there is potential for a memory leak
> since the closed listener will be called on a different key and entries will
> never be evicted from the cache.
> h3. Lifetime expectations
> The expectation of using the core cache key is that it will not change in
> case of deletions, but this is only true on SegmentReader and LeafReader
> impls that delegate to it. Other implementations such as composite readers or
> parallel leaf readers use the same key for "core" and "combined core and
> deletes".
> h3. Throw-away wrappers cause cache trashing
> An application might want to either expose more (with a ParrallelReader or
> MultiReader) or less information (by filtering fields/docs that can be seen)
> depending on the user who is logged in. In that case the application would
> typically maintain a DirectoryReader and then wrap it per request depending
> on the logged user and throw away the wrapper once the request is completed.
> The problem is that these wrappers have their own cache keys and the
> application may build something costly and put it in a cache to throw it away
> a couple milliseconds later. I would rather like for such readers to have a
> way to opt out from caching on order to avoid this performance trap.
> h3. Type safety
> The keys that are exposed are plain java.lang.Object instances, which
> requires caches to look like a {{Map}} which makes it very easy to
> either try to get, put or remove on the wrong object since any object would
> be accepted.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-7410) Make cache keys and closed listeners less trappy
[
https://issues.apache.org/jira/browse/LUCENE-7410?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15890721#comment-15890721
]
ASF subversion and git services commented on LUCENE-7410:
-
Commit 540a23723104e250a4fce94042fb90c86fcf3720 in lucene-solr's branch
refs/heads/master from [~jpountz]
[ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=540a237 ]
LUCENE-7410: Make TestReaderClosed pass if the IndexSearcher wraps a threadpool.
> Make cache keys and closed listeners less trappy
>
>
> Key: LUCENE-7410
> URL: https://issues.apache.org/jira/browse/LUCENE-7410
> Project: Lucene - Core
> Issue Type: Bug
>Reporter: Adrien Grand
> Fix For: master (7.0)
>
> Attachments: LUCENE-7410.patch, LUCENE-7410.patch, LUCENE-7410.patch
>
>
> IndexReader currently exposes getCoreCacheKey(),
> getCombinedCoreAndDeletesKey(), addCoreClosedListener() and
> addReaderClosedListener(). They are typically used to manage resources whose
> lifetime needs to mimic the lifetime of segments/indexes, typically caches.
> I think this is trappy for various reasons:
> h3. Memory leaks
> When maintaining a cache, entries are added to the cache based on the cache
> key and then evicted using the cache key that is given back by the close
> listener, so it is very important that both keys are the same.
> But if a filter reader happens to delegate get*Key() and not
> add*ClosedListener() or vice-versa then there is potential for a memory leak
> since the closed listener will be called on a different key and entries will
> never be evicted from the cache.
> h3. Lifetime expectations
> The expectation of using the core cache key is that it will not change in
> case of deletions, but this is only true on SegmentReader and LeafReader
> impls that delegate to it. Other implementations such as composite readers or
> parallel leaf readers use the same key for "core" and "combined core and
> deletes".
> h3. Throw-away wrappers cause cache trashing
> An application might want to either expose more (with a ParrallelReader or
> MultiReader) or less information (by filtering fields/docs that can be seen)
> depending on the user who is logged in. In that case the application would
> typically maintain a DirectoryReader and then wrap it per request depending
> on the logged user and throw away the wrapper once the request is completed.
> The problem is that these wrappers have their own cache keys and the
> application may build something costly and put it in a cache to throw it away
> a couple milliseconds later. I would rather like for such readers to have a
> way to opt out from caching on order to avoid this performance trap.
> h3. Type safety
> The keys that are exposed are plain java.lang.Object instances, which
> requires caches to look like a {{Map}} which makes it very easy to
> either try to get, put or remove on the wrong object since any object would
> be accepted.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-7410) Make cache keys and closed listeners less trappy
[
https://issues.apache.org/jira/browse/LUCENE-7410?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15890523#comment-15890523
]
Adrien Grand commented on LUCENE-7410:
--
Thanks, I'll dig!
> Make cache keys and closed listeners less trappy
>
>
> Key: LUCENE-7410
> URL: https://issues.apache.org/jira/browse/LUCENE-7410
> Project: Lucene - Core
> Issue Type: Bug
>Reporter: Adrien Grand
> Fix For: master (7.0)
>
> Attachments: LUCENE-7410.patch, LUCENE-7410.patch, LUCENE-7410.patch
>
>
> IndexReader currently exposes getCoreCacheKey(),
> getCombinedCoreAndDeletesKey(), addCoreClosedListener() and
> addReaderClosedListener(). They are typically used to manage resources whose
> lifetime needs to mimic the lifetime of segments/indexes, typically caches.
> I think this is trappy for various reasons:
> h3. Memory leaks
> When maintaining a cache, entries are added to the cache based on the cache
> key and then evicted using the cache key that is given back by the close
> listener, so it is very important that both keys are the same.
> But if a filter reader happens to delegate get*Key() and not
> add*ClosedListener() or vice-versa then there is potential for a memory leak
> since the closed listener will be called on a different key and entries will
> never be evicted from the cache.
> h3. Lifetime expectations
> The expectation of using the core cache key is that it will not change in
> case of deletions, but this is only true on SegmentReader and LeafReader
> impls that delegate to it. Other implementations such as composite readers or
> parallel leaf readers use the same key for "core" and "combined core and
> deletes".
> h3. Throw-away wrappers cause cache trashing
> An application might want to either expose more (with a ParrallelReader or
> MultiReader) or less information (by filtering fields/docs that can be seen)
> depending on the user who is logged in. In that case the application would
> typically maintain a DirectoryReader and then wrap it per request depending
> on the logged user and throw away the wrapper once the request is completed.
> The problem is that these wrappers have their own cache keys and the
> application may build something costly and put it in a cache to throw it away
> a couple milliseconds later. I would rather like for such readers to have a
> way to opt out from caching on order to avoid this performance trap.
> h3. Type safety
> The keys that are exposed are plain java.lang.Object instances, which
> requires caches to look like a {{Map}} which makes it very easy to
> either try to get, put or remove on the wrong object since any object would
> be accepted.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-7410) Make cache keys and closed listeners less trappy
[
https://issues.apache.org/jira/browse/LUCENE-7410?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15890516#comment-15890516
]
Steve Rowe commented on LUCENE-7410:
My Jenkins found a reproducing seed for a
{{TestReaderClosed.testReaderChaining()}} failure, and {{git bisect}} running
the repro line says:
{noformat}
df6f83072303b4891a296b700a50c743284d3c30 is the first bad commit
commit df6f83072303b4891a296b700a50c743284d3c30
Author: Adrien Grand
Date: Tue Feb 28 14:21:30 2017 +0100
LUCENE-7410: Make cache keys and close listeners less trappy.
{noformat}
{noformat}
[junit4] 2> NOTE: reproduce with: ant test -Dtestcase=TestReaderClosed
-Dtests.method=testReaderChaining -Dtests.seed=C4374342D2D99B8F
-Dtests.slow=true -Dtests.locale=hi -Dtests.timezone=America/Dominica
-Dtests.asserts=true -Dtests.file.encoding=US-ASCII
[junit4] FAILURE 0.04s J1 | TestReaderClosed.testReaderChaining <<<
[junit4]> Throwable #1: java.lang.AssertionError: Query failed, but not
due to an AlreadyClosedException
[junit4]>at
__randomizedtesting.SeedInfo.seed([C4374342D2D99B8F:530116CD6543D942]:0)
[junit4]>at
org.apache.lucene.index.TestReaderClosed.testReaderChaining(TestReaderClosed.java:96)
[junit4]>at java.lang.Thread.run(Thread.java:745)
[junit4] 2> NOTE: test params are: codec=Asserting(Lucene70):
{field=PostingsFormat(name=LuceneVarGapFixedInterval)}, docValues:{},
maxPointsInLeafNode=1885, maxMBSortInHeap=6.663525927605304,
sim=RandomSimilarity(queryNorm=true): {}, locale=hi, timezone=America/Dominica
[junit4] 2> NOTE: Linux 4.1.0-custom2-amd64 amd64/Oracle Corporation
1.8.0_77 (64-bit)/cpus=16,threads=1,free=82013152,total=289931264
{noformat}
> Make cache keys and closed listeners less trappy
>
>
> Key: LUCENE-7410
> URL: https://issues.apache.org/jira/browse/LUCENE-7410
> Project: Lucene - Core
> Issue Type: Bug
>Reporter: Adrien Grand
> Fix For: master (7.0)
>
> Attachments: LUCENE-7410.patch, LUCENE-7410.patch, LUCENE-7410.patch
>
>
> IndexReader currently exposes getCoreCacheKey(),
> getCombinedCoreAndDeletesKey(), addCoreClosedListener() and
> addReaderClosedListener(). They are typically used to manage resources whose
> lifetime needs to mimic the lifetime of segments/indexes, typically caches.
> I think this is trappy for various reasons:
> h3. Memory leaks
> When maintaining a cache, entries are added to the cache based on the cache
> key and then evicted using the cache key that is given back by the close
> listener, so it is very important that both keys are the same.
> But if a filter reader happens to delegate get*Key() and not
> add*ClosedListener() or vice-versa then there is potential for a memory leak
> since the closed listener will be called on a different key and entries will
> never be evicted from the cache.
> h3. Lifetime expectations
> The expectation of using the core cache key is that it will not change in
> case of deletions, but this is only true on SegmentReader and LeafReader
> impls that delegate to it. Other implementations such as composite readers or
> parallel leaf readers use the same key for "core" and "combined core and
> deletes".
> h3. Throw-away wrappers cause cache trashing
> An application might want to either expose more (with a ParrallelReader or
> MultiReader) or less information (by filtering fields/docs that can be seen)
> depending on the user who is logged in. In that case the application would
> typically maintain a DirectoryReader and then wrap it per request depending
> on the logged user and throw away the wrapper once the request is completed.
> The problem is that these wrappers have their own cache keys and the
> application may build something costly and put it in a cache to throw it away
> a couple milliseconds later. I would rather like for such readers to have a
> way to opt out from caching on order to avoid this performance trap.
> h3. Type safety
> The keys that are exposed are plain java.lang.Object instances, which
> requires caches to look like a {{Map}} which makes it very easy to
> either try to get, put or remove on the wrong object since any object would
> be accepted.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-7410) Make cache keys and closed listeners less trappy
[
https://issues.apache.org/jira/browse/LUCENE-7410?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15888102#comment-15888102
]
ASF subversion and git services commented on LUCENE-7410:
-
Commit df6f83072303b4891a296b700a50c743284d3c30 in lucene-solr's branch
refs/heads/master from [~jpountz]
[ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=df6f830 ]
LUCENE-7410: Make cache keys and close listeners less trappy.
> Make cache keys and closed listeners less trappy
>
>
> Key: LUCENE-7410
> URL: https://issues.apache.org/jira/browse/LUCENE-7410
> Project: Lucene - Core
> Issue Type: Bug
>Reporter: Adrien Grand
> Attachments: LUCENE-7410.patch, LUCENE-7410.patch, LUCENE-7410.patch
>
>
> IndexReader currently exposes getCoreCacheKey(),
> getCombinedCoreAndDeletesKey(), addCoreClosedListener() and
> addReaderClosedListener(). They are typically used to manage resources whose
> lifetime needs to mimic the lifetime of segments/indexes, typically caches.
> I think this is trappy for various reasons:
> h3. Memory leaks
> When maintaining a cache, entries are added to the cache based on the cache
> key and then evicted using the cache key that is given back by the close
> listener, so it is very important that both keys are the same.
> But if a filter reader happens to delegate get*Key() and not
> add*ClosedListener() or vice-versa then there is potential for a memory leak
> since the closed listener will be called on a different key and entries will
> never be evicted from the cache.
> h3. Lifetime expectations
> The expectation of using the core cache key is that it will not change in
> case of deletions, but this is only true on SegmentReader and LeafReader
> impls that delegate to it. Other implementations such as composite readers or
> parallel leaf readers use the same key for "core" and "combined core and
> deletes".
> h3. Throw-away wrappers cause cache trashing
> An application might want to either expose more (with a ParrallelReader or
> MultiReader) or less information (by filtering fields/docs that can be seen)
> depending on the user who is logged in. In that case the application would
> typically maintain a DirectoryReader and then wrap it per request depending
> on the logged user and throw away the wrapper once the request is completed.
> The problem is that these wrappers have their own cache keys and the
> application may build something costly and put it in a cache to throw it away
> a couple milliseconds later. I would rather like for such readers to have a
> way to opt out from caching on order to avoid this performance trap.
> h3. Type safety
> The keys that are exposed are plain java.lang.Object instances, which
> requires caches to look like a {{Map}} which makes it very easy to
> either try to get, put or remove on the wrong object since any object would
> be accepted.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-7410) Make cache keys and closed listeners less trappy
[
https://issues.apache.org/jira/browse/LUCENE-7410?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15856733#comment-15856733
]
Michael McCandless commented on LUCENE-7410:
+1, this is a great infrastructure API improvement, thanks [~jpountz]. I only
saw some minor issues:
Can/should we {{ensureOpen}} in the {{getReaderCacheHelper}} and
{{addClosedListener}} impls? Just seems like it'd be some good extra
safety to catch accidental mis-use?
There is a typo (like -> live) here:
{noformat}
+// TODO: this is trappy as the expectation is that core keys like for a
long
{noformat}
And a missing d in StandarDirectoryReader in the exception message here:
{noformat}
+if (cacheHelper == null) {
+ throw new IllegalStateException("StandarDirectoryReader must support
caching");
+}
{noformat}
And extra s here:
{noformat}
+ * valuess updates may be considered equal. Consider using
{noformat}
> Make cache keys and closed listeners less trappy
>
>
> Key: LUCENE-7410
> URL: https://issues.apache.org/jira/browse/LUCENE-7410
> Project: Lucene - Core
> Issue Type: Bug
>Reporter: Adrien Grand
> Attachments: LUCENE-7410.patch, LUCENE-7410.patch, LUCENE-7410.patch
>
>
> IndexReader currently exposes getCoreCacheKey(),
> getCombinedCoreAndDeletesKey(), addCoreClosedListener() and
> addReaderClosedListener(). They are typically used to manage resources whose
> lifetime needs to mimic the lifetime of segments/indexes, typically caches.
> I think this is trappy for various reasons:
> h3. Memory leaks
> When maintaining a cache, entries are added to the cache based on the cache
> key and then evicted using the cache key that is given back by the close
> listener, so it is very important that both keys are the same.
> But if a filter reader happens to delegate get*Key() and not
> add*ClosedListener() or vice-versa then there is potential for a memory leak
> since the closed listener will be called on a different key and entries will
> never be evicted from the cache.
> h3. Lifetime expectations
> The expectation of using the core cache key is that it will not change in
> case of deletions, but this is only true on SegmentReader and LeafReader
> impls that delegate to it. Other implementations such as composite readers or
> parallel leaf readers use the same key for "core" and "combined core and
> deletes".
> h3. Throw-away wrappers cause cache trashing
> An application might want to either expose more (with a ParrallelReader or
> MultiReader) or less information (by filtering fields/docs that can be seen)
> depending on the user who is logged in. In that case the application would
> typically maintain a DirectoryReader and then wrap it per request depending
> on the logged user and throw away the wrapper once the request is completed.
> The problem is that these wrappers have their own cache keys and the
> application may build something costly and put it in a cache to throw it away
> a couple milliseconds later. I would rather like for such readers to have a
> way to opt out from caching on order to avoid this performance trap.
> h3. Type safety
> The keys that are exposed are plain java.lang.Object instances, which
> requires caches to look like a {{Map}} which makes it very easy to
> either try to get, put or remove on the wrong object since any object would
> be accepted.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-7410) Make cache keys and closed listeners less trappy
[
https://issues.apache.org/jira/browse/LUCENE-7410?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15855038#comment-15855038
]
Yonik Seeley commented on LUCENE-7410:
--
Seems like CacheHelper is missing from the patch?
> Make cache keys and closed listeners less trappy
>
>
> Key: LUCENE-7410
> URL: https://issues.apache.org/jira/browse/LUCENE-7410
> Project: Lucene - Core
> Issue Type: Bug
>Reporter: Adrien Grand
> Attachments: LUCENE-7410.patch, LUCENE-7410.patch, LUCENE-7410.patch
>
>
> IndexReader currently exposes getCoreCacheKey(),
> getCombinedCoreAndDeletesKey(), addCoreClosedListener() and
> addReaderClosedListener(). They are typically used to manage resources whose
> lifetime needs to mimic the lifetime of segments/indexes, typically caches.
> I think this is trappy for various reasons:
> h3. Memory leaks
> When maintaining a cache, entries are added to the cache based on the cache
> key and then evicted using the cache key that is given back by the close
> listener, so it is very important that both keys are the same.
> But if a filter reader happens to delegate get*Key() and not
> add*ClosedListener() or vice-versa then there is potential for a memory leak
> since the closed listener will be called on a different key and entries will
> never be evicted from the cache.
> h3. Lifetime expectations
> The expectation of using the core cache key is that it will not change in
> case of deletions, but this is only true on SegmentReader and LeafReader
> impls that delegate to it. Other implementations such as composite readers or
> parallel leaf readers use the same key for "core" and "combined core and
> deletes".
> h3. Throw-away wrappers cause cache trashing
> An application might want to either expose more (with a ParrallelReader or
> MultiReader) or less information (by filtering fields/docs that can be seen)
> depending on the user who is logged in. In that case the application would
> typically maintain a DirectoryReader and then wrap it per request depending
> on the logged user and throw away the wrapper once the request is completed.
> The problem is that these wrappers have their own cache keys and the
> application may build something costly and put it in a cache to throw it away
> a couple milliseconds later. I would rather like for such readers to have a
> way to opt out from caching on order to avoid this performance trap.
> h3. Type safety
> The keys that are exposed are plain java.lang.Object instances, which
> requires caches to look like a {{Map}} which makes it very easy to
> either try to get, put or remove on the wrong object since any object would
> be accepted.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-7410) Make cache keys and closed listeners less trappy
[
https://issues.apache.org/jira/browse/LUCENE-7410?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15419068#comment-15419068
]
Robert Muir commented on LUCENE-7410:
-
At a glance a lot of these ideas sound great, but i need some time to look at
the patch in detail, i only glanced thru it.
things like switching the filterreader default to not caching at all, well
thats arguably less trappy, but at the same time a risky change. Are all of our
filter readers really correct/optimal now? Maybe its just better for these
classes to redeclare the method as 'abstract', forcing implementations to deal
with it? Personally I think thats the best solution for this case.
Not a huge fan of the name CacheHelper, at the same time I don't have a better
suggestion.
> Make cache keys and closed listeners less trappy
>
>
> Key: LUCENE-7410
> URL: https://issues.apache.org/jira/browse/LUCENE-7410
> Project: Lucene - Core
> Issue Type: Bug
>Reporter: Adrien Grand
> Attachments: LUCENE-7410.patch
>
>
> IndexReader currently exposes getCoreCacheKey(),
> getCombinedCoreAndDeletesKey(), addCoreClosedListener() and
> addReaderClosedListener(). They are typically used to manage resources whose
> lifetime needs to mimic the lifetime of segments/indexes, typically caches.
> I think this is trappy for various reasons:
> h3. Memory leaks
> When maintaining a cache, entries are added to the cache based on the cache
> key and then evicted using the cache key that is given back by the close
> listener, so it is very important that both keys are the same.
> But if a filter reader happens to delegate get*Key() and not
> add*ClosedListener() or vice-versa then there is potential for a memory leak
> since the closed listener will be called on a different key and entries will
> never be evicted from the cache.
> h3. Lifetime expectations
> The expectation of using the core cache key is that it will not change in
> case of deletions, but this is only true on SegmentReader and LeafReader
> impls that delegate to it. Other implementations such as composite readers or
> parallel leaf readers use the same key for "core" and "combined core and
> deletes".
> h3. Throw-away wrappers cause cache trashing
> An application might want to either expose more (with a ParrallelReader or
> MultiReader) or less information (by filtering fields/docs that can be seen)
> depending on the user who is logged in. In that case the application would
> typically maintain a DirectoryReader and then wrap it per request depending
> on the logged user and throw away the wrapper once the request is completed.
> The problem is that these wrappers have their own cache keys and the
> application may build something costly and put it in a cache to throw it away
> a couple milliseconds later. I would rather like for such readers to have a
> way to opt out from caching on order to avoid this performance trap.
> h3. Type safety
> The keys that are exposed are plain java.lang.Object instances, which
> requires caches to look like a {{Map}} which makes it very easy to
> either try to get, put or remove on the wrong object since any object would
> be accepted.
--
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] (LUCENE-7410) Make cache keys and closed listeners less trappy
[
https://issues.apache.org/jira/browse/LUCENE-7410?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15419055#comment-15419055
]
Adrien Grand commented on LUCENE-7410:
--
Any opinions?
> Make cache keys and closed listeners less trappy
>
>
> Key: LUCENE-7410
> URL: https://issues.apache.org/jira/browse/LUCENE-7410
> Project: Lucene - Core
> Issue Type: Bug
>Reporter: Adrien Grand
> Attachments: LUCENE-7410.patch
>
>
> IndexReader currently exposes getCoreCacheKey(),
> getCombinedCoreAndDeletesKey(), addCoreClosedListener() and
> addReaderClosedListener(). They are typically used to manage resources whose
> lifetime needs to mimic the lifetime of segments/indexes, typically caches.
> I think this is trappy for various reasons:
> h3. Memory leaks
> When maintaining a cache, entries are added to the cache based on the cache
> key and then evicted using the cache key that is given back by the close
> listener, so it is very important that both keys are the same.
> But if a filter reader happens to delegate get*Key() and not
> add*ClosedListener() or vice-versa then there is potential for a memory leak
> since the closed listener will be called on a different key and entries will
> never be evicted from the cache.
> h3. Lifetime expectations
> The expectation of using the core cache key is that it will not change in
> case of deletions, but this is only true on SegmentReader and LeafReader
> impls that delegate to it. Other implementations such as composite readers or
> parallel leaf readers use the same key for "core" and "combined core and
> deletes".
> h3. Throw-away wrappers cause cache trashing
> An application might want to either expose more (with a ParrallelReader or
> MultiReader) or less information (by filtering fields/docs that can be seen)
> depending on the user who is logged in. In that case the application would
> typically maintain a DirectoryReader and then wrap it per request depending
> on the logged user and throw away the wrapper once the request is completed.
> The problem is that these wrappers have their own cache keys and the
> application may build something costly and put it in a cache to throw it away
> a couple milliseconds later. I would rather like for such readers to have a
> way to opt out from caching on order to avoid this performance trap.
> h3. Type safety
> The keys that are exposed are plain java.lang.Object instances, which
> requires caches to look like a {{Map}} which makes it very easy to
> either try to get, put or remove on the wrong object since any object would
> be accepted.
--
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] (LUCENE-7410) Make cache keys and closed listeners less trappy
[
https://issues.apache.org/jira/browse/LUCENE-7410?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15414994#comment-15414994
]
Adrien Grand commented on LUCENE-7410:
--
Agreed. I'm also looking into removing the ability to remove closed listeners.
> Make cache keys and closed listeners less trappy
>
>
> Key: LUCENE-7410
> URL: https://issues.apache.org/jira/browse/LUCENE-7410
> Project: Lucene - Core
> Issue Type: Bug
>Reporter: Adrien Grand
>
> IndexReader currently exposes getCoreCacheKey(),
> getCombinedCoreAndDeletesKey(), addCoreClosedListener() and
> addReaderClosedListener(). They are typically used to manage resources whose
> lifetime needs to mimic the lifetime of segments/indexes, typically caches.
> I think this is trappy for various reasons:
> h3. Memory leaks
> When maintaining a cache, entries are added to the cache based on the cache
> key and then evicted using the cache key that is given back by the close
> listener, so it is very important that both keys are the same.
> But if a filter reader happens to delegate get*Key() and not
> add*ClosedListener() or vice-versa then there is potential for a memory leak
> since the closed listener will be called on a different key and entries will
> never be evicted from the cache.
> h3. Lifetime expectations
> The expectation of using the core cache key is that it will not change in
> case of deletions, but this is only true on SegmentReader and LeafReader
> impls that delegate to it. Other implementations such as composite readers or
> parallel leaf readers use the same key for "core" and "combined core and
> deletes".
> h3. Throw-away wrappers cause cache trashing
> An application might want to either expose more (with a ParrallelReader or
> MultiReader) or less information (by filtering fields/docs that can be seen)
> depending on the user who is logged in. In that case the application would
> typically maintain a DirectoryReader and then wrap it per request depending
> on the logged user and throw away the wrapper once the request is completed.
> The problem is that these wrappers have their own cache keys and the
> application may build something costly and put it in a cache to throw it away
> a couple milliseconds later. I would rather like for such readers to have a
> way to opt out from caching on order to avoid this performance trap.
> h3. Type safety
> The keys that are exposed are plain java.lang.Object instances, which
> requires caches to look like a {{Map}} which makes it very easy to
> either try to get, put or remove on the wrong object since any object would
> be accepted.
--
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] (LUCENE-7410) Make cache keys and closed listeners less trappy
[
https://issues.apache.org/jira/browse/LUCENE-7410?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15414942#comment-15414942
]
Robert Muir commented on LUCENE-7410:
-
{{getCombinedCoreAndDeletesKey()}}: what uses this one? Can we remove it?
> Make cache keys and closed listeners less trappy
>
>
> Key: LUCENE-7410
> URL: https://issues.apache.org/jira/browse/LUCENE-7410
> Project: Lucene - Core
> Issue Type: Bug
>Reporter: Adrien Grand
>
> IndexReader currently exposes getCoreCacheKey(),
> getCombinedCoreAndDeletesKey(), addCoreClosedListener() and
> addReaderClosedListener(). They are typically used to manage resources whose
> lifetime needs to mimic the lifetime of segments/indexes, typically caches.
> I think this is trappy for various reasons:
> h3. Memory leaks
> When maintaining a cache, entries are added to the cache based on the cache
> key and then evicted using the cache key that is given back by the close
> listener, so it is very important that both keys are the same.
> But if a filter reader happens to delegate get*Key() and not
> add*ClosedListener() or vice-versa then there is potential for a memory leak
> since the closed listener will be called on a different key and entries will
> never be evicted from the cache.
> h3. Lifetime expectations
> The expectation of using the core cache key is that it will not change in
> case of deletions, but this is only true on SegmentReader and LeafReader
> impls that delegate to it. Other implementations such as composite readers or
> parallel leaf readers use the same key for "core" and "combined core and
> deletes".
> h3. Throw-away wrappers cause cache trashing
> An application might want to either expose more (with a ParrallelReader or
> MultiReader) or less information (by filtering fields/docs that can be seen)
> depending on the user who is logged in. In that case the application would
> typically maintain a DirectoryReader and then wrap it per request depending
> on the logged user and throw away the wrapper once the request is completed.
> The problem is that these wrappers have their own cache keys and the
> application may build something costly and put it in a cache to throw it away
> a couple milliseconds later. I would rather like for such readers to have a
> way to opt out from caching on order to avoid this performance trap.
> h3. Type safety
> The keys that are exposed are plain java.lang.Object instances, which
> requires caches to look like a {{Map}} which makes it very easy to
> either try to get, put or remove on the wrong object since any object would
> be accepted.
--
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
