You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "Chris M. Hostetter (Jira)" <ji...@apache.org> on 2019/11/08 18:34:00 UTC
[jira] [Commented] (SOLR-13909) Everything about CheckBackupStatus
is broken
[ https://issues.apache.org/jira/browse/SOLR-13909?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16970510#comment-16970510 ]
Chris M. Hostetter commented on SOLR-13909:
-------------------------------------------
I've got some simpler/saner code in the new {{TestStressThreadBackup}} I'm adding as part of SOLR-13872 that i'll refactor and re-use in all places that currently dela with CheckBackupStatus once I'm done with that jira.
> Everything about CheckBackupStatus is broken
> --------------------------------------------
>
> Key: SOLR-13909
> URL: https://issues.apache.org/jira/browse/SOLR-13909
> Project: Solr
> Issue Type: Test
> Security Level: Public(Default Security Level. Issues are Public)
> Reporter: Chris M. Hostetter
> Assignee: Chris M. Hostetter
> Priority: Major
>
> While working on SOLR-13872 I tried to take advantage of the existing {{CheckBackupStatus}} helper class and discovered that just about every aspect of this class is broken and needs fixed:
> * doesn't use SolrClient, pulls out it's URL to do a bare HTTP request
> * hardcoded assumption of xml - but doesn't parse it just tries to match regexes against it
> * almost every usage of this class follows the same broken "loop" pattern that garuntees the test will sleep more then it needs to even after {{CheckBackupStatus}} thinks the backup is a success...
> {code:java}
> CheckBackupStatus checkBackupStatus = new CheckBackupStatus(...);
> while (!checkBackupStatus.success) {
> checkBackupStatus.fetchStatus();
> Thread.sleep(1000);
> }
> {code}
> * the 3 arg constructor is broken both in design and in implementation:
> ** it appears to be useful for checking that a _new_ backup has succeeded after a {{lastBackupTimestamp}} from some previously successful check
> ** in reality it only ever reports {{success}} if it's status check indicates the most recent backup has the exact {{.equals()}} time stamp as {{lastBackupTimestamp}}
> ** *AND THESE TIMESTAMPS ONLY HAVE MINUTE PRECISION*
> ** As far as i can tell, the only the tests using the 3 arg version ever pass is because of the broken loop pattern:
> *** they ask for the status so quick, it's either already done (during the same wall clock minute) or it's not done yet and they re-read the "old" status (with the old matching timestamp)
> *** either way, the test then sleeps for a second giving the "new" backup enough time to actually finish
> ** AFAICT if the System clock ticks over to a new minute in between these sleep calls, the test is garunteed to loop forever!
> ----
> Everything about this class needs to die and be replaced with something better.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org