[jira] [Updated] (SOLR-13700) Race condition in initializing metrics for new security plugins when security.json is modified

2019-08-16 Thread Hoss Man (JIRA)


 [ 
https://issues.apache.org/jira/browse/SOLR-13700?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Hoss Man updated SOLR-13700:

Attachment: SOLR-13700.patch
Status: Open  (was: Open)


I've updated the patch to move 
{{pkiAuthenticationPlugin.initializeMetrics((...)}} so that it is called 
exactly once immediately after {{new PKIAuthenticationPlugin(...)}}.

precommit now passes, i'm still beasting.



[~janhoy]: I don't understand this part of your comment...

bq. ... Re point 2, your patch deletes the wrong lines for auditloggerPlugin 
metrics. ...

The only lines modified in my patch(es) that mention {{auditloggerPlugin}} is 
to move the {{auditloggerPlugin.plugin.initializeMetrics(...)}} call into the 
existing {{initializeAuditloggerPlugin(...)}} as per the point of this jira.

Can you please elaborate on what lines you think were wrong to be deleted/moved 
?  ... ideally with a counter-patch, or suggested new case demonstrating the 
problem, so there's no ambiguity as to what you mean?


> Race condition in initializing metrics for new security plugins when 
> security.json is modified
> --
>
> Key: SOLR-13700
> URL: https://issues.apache.org/jira/browse/SOLR-13700
> Project: Solr
>  Issue Type: Bug
>  Security Level: Public(Default Security Level. Issues are Public) 
>Reporter: Hoss Man
>Assignee: Hoss Man
>Priority: Major
> Attachments: SOLR-13700.patch, SOLR-13700.patch
>
>
> When new security plugins are initialized due to remote API requetss, there 
> is a delay between "registering" the new plugins for use in methods like 
> {{initializeAuthenticationPlugin()}} (by assigning them to CoreContainer's 
> volatile {{this.authenticationPlugin}} variable) and when the 
> {{initializeMetrics(..)}} method is called on these plugins, so that they 
> continue to use the existing {{Metric}} instances as the plugins they are 
> replacing.
> Because these security plugins maintain local refrences to these Metrics (and 
> don't "get" them from the MetricRegisty everytime they need to {{inc()}} 
> them) this means that there is short race condition situation such that 
> during the window of time after a new plugin instance is put into use, but 
> before  {{initializeMetrics(..)}} is called on them, at these plugins are 
> responsible for accepting/rejecting requests, and decisions in {{Metric}} 
> instances that are not registered and subsequently get thrown away (and GCed) 
> once the CoreContainer gets around to calling {{initializeMetrics(..)}} (and 
> the plugin starts using the pre-existing metric objects)
> 
> This has some noticible impacts on auth tests on CPU constrained jenkins 
> machines (even after putting in place SOLR-13464 work arounds) that make 
> assertions about the metrics recorded.
> In real world situations, the impact of this bug on users is minor: for a few 
> micro/milli-seconds, requests may come in w/o being counted in the auth 
> metrics -- which may also result in descrepencies between the auth metrics 
> totals and the overall request metrics.  



--
This message was sent by Atlassian JIRA
(v7.6.14#76016)

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



[jira] [Updated] (SOLR-13700) Race condition in initializing metrics for new security plugins when security.json is modified

2019-08-15 Thread Hoss Man (JIRA)


 [ 
https://issues.apache.org/jira/browse/SOLR-13700?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Hoss Man updated SOLR-13700:

  Assignee: Hoss Man
Attachment: SOLR-13700.patch
Status: Open  (was: Open)


Attaching a patch to address this, one nocommit regarding something that makes 
no sense to me...

[~ab] - can you please review and sanity check:

# that my understanding is correct, and that it's "safe" (and more correct) for 
the "old" and "new" instances of these plugins to be using the same Metric 
instances before the "new" plugin replaces the old one
# the nocommit comments -- unless i'm missing something 
{{reloadSecurityProperties()}} has no business calling 
{{pkiAuthenticationPlugin.initializeMetrics(...)}}, because 
{{pkiAuthenticationPlugin}} can never change as a result of reloading the 
security.json ... so {{pkiAuthenticationPlugin.initializeMetrics(...)}} should 
be called exactly once (and only once) for it's entire lifecycle ... ideally in 
{{CoreContainer.load()}} immediately after calling {{pkiAuthenticationPlugin = 
new PKIAuthenticationPlugin...)}}



> Race condition in initializing metrics for new security plugins when 
> security.json is modified
> --
>
> Key: SOLR-13700
> URL: https://issues.apache.org/jira/browse/SOLR-13700
> Project: Solr
>  Issue Type: Bug
>  Security Level: Public(Default Security Level. Issues are Public) 
>Reporter: Hoss Man
>Assignee: Hoss Man
>Priority: Major
> Attachments: SOLR-13700.patch
>
>
> When new security plugins are initialized due to remote API requetss, there 
> is a delay between "registering" the new plugins for use in methods like 
> {{initializeAuthenticationPlugin()}} (by assigning them to CoreContainer's 
> volatile {{this.authenticationPlugin}} variable) and when the 
> {{initializeMetrics(..)}} method is called on these plugins, so that they 
> continue to use the existing {{Metric}} instances as the plugins they are 
> replacing.
> Because these security plugins maintain local refrences to these Metrics (and 
> don't "get" them from the MetricRegisty everytime they need to {{inc()}} 
> them) this means that there is short race condition situation such that 
> during the window of time after a new plugin instance is put into use, but 
> before  {{initializeMetrics(..)}} is called on them, at these plugins are 
> responsible for accepting/rejecting requests, and decisions in {{Metric}} 
> instances that are not registered and subsequently get thrown away (and GCed) 
> once the CoreContainer gets around to calling {{initializeMetrics(..)}} (and 
> the plugin starts using the pre-existing metric objects)
> 
> This has some noticible impacts on auth tests on CPU constrained jenkins 
> machines (even after putting in place SOLR-13464 work arounds) that make 
> assertions about the metrics recorded.
> In real world situations, the impact of this bug on users is minor: for a few 
> micro/milli-seconds, requests may come in w/o being counted in the auth 
> metrics -- which may also result in descrepencies between the auth metrics 
> totals and the overall request metrics.  



--
This message was sent by Atlassian JIRA
(v7.6.14#76016)

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