[
https://issues.apache.org/jira/browse/SOLR-13420?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16830371#comment-16830371
]
Gus Heck commented on SOLR-13420:
---------------------------------
(moving back from SOLR-13418)
{quote}The leak comes in the case of collections getting moved out of the node
or with collections being created/deleted. It’s not only a memory leak, but
also a ZooKeeper watch leak.
{quote}
Ah this is a good point which I hadn't considered! So the patch I'm attaching
now should solve this It uses a more interesting cache that allows unused
entries to age out, but only if there's no watcher. Since process() doesn't
call refreshAndWatch if there are no properties currently cached, the watch
will not get renewed allowing both the used memory and the watch to expire.
{quote}Why is TimeRoutedAliasUpdateProcessor created out of a static wrap
function in DUP instead of a Factory?
{quote}
In the very early days of designing time routed aliases, [~dsmiley] and I were
contemplating whether or not the feature should be a modification to
DistributedUpdateRequestProcessor or a helper class inside
DistributedUpdateRequestProcessor but neither option seemed attractive in that
either way the already overly large and complicated
DistributedUpdateRequestProcessor.java file would gain further complexity. Dave
came up with the wrap() method idea and I like it because it segregates all the
alias based routing logic out of DURP which otherwise doesn't care about it
while ensuring that nothing can come between the TRA code and DURP (which would
be problematic because the TRA URP offers an optimized code path when it knows
exactly where a time routed request should go, and takes over for DURP to avoid
an extra hop). TRA URP is conceptually just extended functionality for DURP.
{quote}
{code:java}
- if (!collectionPropsWatches.containsKey(coll)) {
+ if (!watchedCollectionProps.containsKey(coll)) {
{code}
With this change, if the collection is not being watched, a collection property
watcher would not be notified.
{quote}
I'm not sure I understand your concern. It doesn't sound like a problem if a
watcher isn't notified when nobody is watching?
{quote}Also, since the {{collectionPropsWatches}} would still contain the
collection name, the value of the properties in the map would go stale. What’s
the intention of this change?
{quote}
Quite the opposite, here's the full context:
{code:java}
@Override
public void process(WatchedEvent event) {
// session events are not change events, and do not remove the watcher
if (EventType.None.equals(event.getType())) {
return;
}
if (!watchedCollectionProps.containsKey(coll)) {
// No one can be notified of the change, we can ignore it and "unset"
the watch
// by not refreshing.
log.debug("Ignoring property change for collection {}, zk watch will
not be renewed", coll);
return;
}
log.debug("A collection property change: [{}] for collection [{}] has
occurred - updating...",
event, coll);
refreshAndWatch(true);
} {code}
So if there is a set of properties cached, they are refreshed, and if not, they
are not. (comments and logging shown here are updated as in patch 2)
> Allow Routed Aliases to use Collection Properties instead of core properties
> ----------------------------------------------------------------------------
>
> Key: SOLR-13420
> URL: https://issues.apache.org/jira/browse/SOLR-13420
> Project: Solr
> Issue Type: Sub-task
> Security Level: Public(Default Security Level. Issues are Public)
> Components: SolrCloud
> Affects Versions: master (9.0)
> Reporter: Gus Heck
> Assignee: Gus Heck
> Priority: Major
> Attachments: SOLR-13420.patch
>
>
> The current routed alias code is relying on a core property named
> routedAliasName to detect when the Routed Alias wrapper URP should be applied
> to Distributed Update Request Processor.
> {code:java}
> #Written by CorePropertiesLocator
> #Sun Mar 03 06:21:14 UTC 2019
> routedAliasName=testalias21
> numShards=2
> collection.configName=_default
> ... etc...
> {code}
> Core properties are not changeable after the core is created, and they are
> written to the file system for every core. To support a unit test for
> SOLR-13419 I need to create some legacy formatted collection names, and
> arrange them into a TRA, but this is impossible due to the fact that I can't
> change the core property from the test. There's a TODO dating back to the
> original TRA implementation in the routed alias code to switch to collection
> properties instead, so this ticket will address that TOD to support the test
> required for SOLR-13419.
> Back compatibility with legacy core based TRA's and CRA's will of course be
> maintained. I also expect that this will facilitate some more nimble handling
> or routed aliases with future auto-scaling capabilities such as possibly
> detaching and archiving collections to cheaper, slower machines rather than
> deleting them. (presently such a collection would still attempt to use the
> routed alias if it received an update even if it were no longer in the list
> of collections for the alias)
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
