lucene-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF subversion and git services (Jira)" <j...@apache.org>
Subject [jira] [Commented] (SOLR-13909) Everything about CheckBackupStatus is broken
Date Fri, 15 Nov 2019 17:34:00 GMT

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

ASF subversion and git services commented on SOLR-13909:
--------------------------------------------------------

Commit 95e791622b97cd9e7ea59e748d287c3e7f17c76e in lucene-solr's branch refs/heads/branch_8x
from Chris M. Hostetter
[ https://gitbox.apache.org/repos/asf?p=lucene-solr.git;h=95e7916 ]

SOLR-13909: ReplicationHandler testing: Replace the completely broken CheckBackupStatus with
a new BackupStatusChecker helper class

(cherry picked from commit 805305c41046da3385d2146de290e52c556b373c)


> 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
>         Attachments: SOLR-13909.patch
>
>
> 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


Mime
View raw message