You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by "Jan Høydahl (JIRA)" <ji...@apache.org> on 2019/08/16 09:22:00 UTC
[jira] [Commented] (SOLR-13701) JWTAuthPlugin calls
authenticationFailure (which calls HttpServletResponsesendError) before
updating metrics - breaks tests
[ https://issues.apache.org/jira/browse/SOLR-13701?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16908886#comment-16908886 ]
Jan Høydahl commented on SOLR-13701:
------------------------------------
Updating metrics before returning sounds reasonable since we assert on those counts (which has proven fragile).
I don't see the motivation to remove the Test-code 2s retry yet, unless extensive beasting proves that it is no longer needed?
> JWTAuthPlugin calls authenticationFailure (which calls HttpServletResponsesendError) before updating metrics - breaks tests
> ---------------------------------------------------------------------------------------------------------------------------
>
> Key: SOLR-13701
> URL: https://issues.apache.org/jira/browse/SOLR-13701
> Project: Solr
> Issue Type: Bug
> Security Level: Public(Default Security Level. Issues are Public)
> Reporter: Hoss Man
> Assignee: Hoss Man
> Priority: Major
> Attachments: SOLR-13701.patch
>
>
> The way JWTAuthPlugin is currently implemented, any failures are sent to the remote client (via {{authenticationFailure(...)}} which calls {{HttpServletResponsesendError(...)}}) *before* {{JWTAuthPlugin.doAuthenticate(...)}} has a chance to update it's metrics (like {{numErrors}} and {{numWrongCredentials}})
> This causes a race condition in tests where test threads can:
> * see an error response/Exception before the server thread has updated metrics (like {{numErrors}} and {{numWrongCredentials}})
> * call white box methods like {{SolrCloudAuthTestCase.assertAuthMetricsMinimums(...)}} to assert expected metrics
> ...all before the server thread has ever gotten around to being able to update the metrics in question.
> {{SolrCloudAuthTestCase.assertAuthMetricsMinimums(...)}} currently has some {{"First metrics count assert failed, pausing 2s before re-attempt"}} evidently to try and work around this bug, but it's still no garuntee that the server thread will be scheduled before the retry happens.
> We can/should just fix JWTAuthPlugin to ensure the metrics are updated before {{authenticationFailure(...)}} is called, and then remove the "pausing 2s before re-attempt" logic from {{SolrCloudAuthTestCase}} - between this bug fix, and the existing work around for SOLR-13464, there should be absolutely no reason to "retry" reading hte metrics.
> (NOTE: BasicAuthPlugin has a similar {{authenticationFailure(...)}} method that also calls {{HttpServletResponse.sendError(...)}} - but it already (correctly) updates the error/failure metrics *before* calling that method.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org