[jira] [Comment Edited] (SOLR-11925) Auto delete oldest collections in a time routed alias

["Gus Heck (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/SOLR-11925?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16356438#comment-16356438 ] 

Gus Heck edited comment on SOLR-11925 at 2/8/18 3:31 AM:
---------------------------------------------------------

Took a look at the patch, here are my thoughts:

Like:
 * rename of handleResponse
 * fluent style for addMetadata (returning the command)
 * create and delete sections of results :)
 * Nice cleanup in invokeAction, especially collecting and propagating the exception earlier.

Question:
 * Why is remoteInvoke on RouedAliasCreateCollectionCommand when it's only used from TimeRoutedAliasUpdateProcessor?
 * Rename field on zkstateReader aliasesHolder to aliasesManager?
 * You added 2 parseSolrDateToInstant() methods with different params, but neither seems to be used?
 * in your cleanup of invokeAction etc, you seem to have dropped some conditionals looking for null or -1 error codes, which was added by MarkMiller in 2013... (bottom of handleResponse, which is now sentToOCPQueue) Any idea what that was trapping? Why is it not needed now?

Thought about:
 * Thinking about the validation in TimeRoutedAlias ... Does seem that if an alias metadata gets messed up causing validation issue, all operations sending data to that alias will start failing. This worried me initially, but is probably ok. Must not get used in ModifyAlias in the future, since that would make it impossible to repair broken metadata. Not something we are doing now, but a potential pitfall. In constructor validation of values often worries me due to the inability to model busted state if state becomes busted.
 * We seem to rely on applyModificationandExportToZk to ensure that the alias is updated before deleting collections, but I think that's a little risky given the need for zookeeper watches to update. If we delete collections quickly after that nodes that have not processed watch notifications may send to the deleted shard? Do we need another (ugly, but magic) 100ms wait
 * I think we can worry about making deletion async if that becomes a pain point later. Simple to start as you have it now is good.

Suggest:
 * Javadoc for deleteCollectionsAndUpdateAlias() - especially explanation of the correct notion for the "from" parameter.
 * Similarly doc for autoDeleteBeforeTime property on TimeRoutedAlias... time stuff is always fiddly, helps to have some doc. Not sure why it's not autoDeleteAfterInterval? Maybe you mean autoDeleteIfBefore?


was (Author: gus_heck):
Took a look at the patch, here are my thoughts:

Like:
 * rename of handle response
 * fluent style for addMetadata (returning the command)
 * create and delete sections of results :)
 * Nice cleanup in invokeAction, especially collecting and propagating the exception earlier.

Question:
 * Why is remoteInvoke on RouedAliasCreateCollectionCommand when it's only used from TimeRoutedAliasUpdateProcessor?
 * Rename field on zkstateReader aliasesHolder to aliasesManager?
 * You added 2 parseSolrDateToInstant() methods with different params, but neither seems to be used?
 * in your cleanup of invokeAction etc, you seem to have dropped some conditionals looking for null or -1 error codes, which was added by MarkMiller in 2013... (bottom of handleResponse, which is now sentToOCPQueue) Any idea what that was trapping? Why is it not needed now?

Thought about:
 * Thinking about the validation in TimeRoutedAlias ... Does seem that if an alias metadata gets messed up causing validation issue, all operations sending data to that alias will start failing. This worried me initially, but is probably ok. Must not get used in ModifyAlias in the future, since that would make it impossible to repair broken metadata. Not something we are doing now, but a potential pitfall. In constructor validation of values often worries me due to the inability to model busted state if state becomes busted.
 * We seem to rely on applyModificationandExportToZk to ensure that the alias is updated before deleting collections, but I think that's a little risky given the need for zookeeper watches to update. If we delete collections quickly after that nodes that have not processed watch notifications may send to the deleted shard? Do we need another (ugly, but magic) 100ms wait
 * I think we can worry about making deletion async if that becomes a pain point later. Simple to start as you have it now is good.

Suggest:
 * Javadoc for deleteCollectionsAndUpdateAlias() - especially explanation of the correct notion for the "from" parameter.
 * Similarly doc for autoDeleteBeforeTime property on TimeRoutedAlias... time stuff is always fiddly, helps to have some doc. Not sure why it's not autoDeleteAfterInterval? Maybe you mean autoDeleteIfBefore?

> Auto delete oldest collections in a time routed alias
> -----------------------------------------------------
>
>                 Key: SOLR-11925
>                 URL: https://issues.apache.org/jira/browse/SOLR-11925
>             Project: Solr
>          Issue Type: Sub-task
>      Security Level: Public(Default Security Level. Issues are Public) 
>          Components: SolrCloud
>            Reporter: David Smiley
>            Assignee: David Smiley
>            Priority: Major
>             Fix For: 7.3
>
>         Attachments: SOLR-11925.patch
>
>
> The oldest collections in a Time Routed Alias should be automatically deleted, according to a new alias metadata that establishes how long.  It can be checked as new data flows in at TimeRoutedAliasUpdateProcessor and thus it won't occur if new data isn't coming in.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org