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

2019-08-18 Thread ASF subversion and git services (JIRA)


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

ASF subversion and git services commented on SOLR-13700:


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

SOLR-13700: Fixed a race condition when initializing metrics for new security 
plugins on security.json change

(cherry picked from commit 251259d5abf94578b51da3efd05327da72667e7e)


> 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] [Commented] (SOLR-13700) Race condition in initializing metrics for new security plugins when security.json is modified

2019-08-18 Thread ASF subversion and git services (JIRA)


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

ASF subversion and git services commented on SOLR-13700:


Commit 251259d5abf94578b51da3efd05327da72667e7e in lucene-solr's branch 
refs/heads/master from Chris M. Hostetter
[ https://gitbox.apache.org/repos/asf?p=lucene-solr.git;h=251259d ]

SOLR-13700: Fixed a race condition when initializing metrics for new security 
plugins on security.json change


> 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] [Commented] (SOLR-13700) Race condition in initializing metrics for new security plugins when security.json is modified

2019-08-16 Thread Andrzej Bialecki (JIRA)


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

Andrzej Bialecki  commented on SOLR-13700:
--

+1.

> 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] [Commented] (SOLR-13700) Race condition in initializing metrics for new security plugins when security.json is modified

2019-08-16 Thread JIRA


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

Jan Høydahl commented on SOLR-13700:


Forget my comment, I now see what happened in the first patch. And the newest 
patch makes the code easier to read, I like it!

> 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] [Commented] (SOLR-13700) Race condition in initializing metrics for new security plugins when security.json is modified

2019-08-16 Thread Andrzej Bialecki (JIRA)


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

Andrzej Bialecki  commented on SOLR-13700:
--

[~janhoy] created SOLR-13702.

> 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



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

2019-08-16 Thread JIRA


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

Jan Høydahl commented on SOLR-13700:


{quote}Also, I just spotted that both {{AuditLoggerPlugin.initializeMetrics}} 
and {{AuthenticationPlugin.initializeMetrics}}needlessly add metrics names to 
{{metricNames}} - this is already done as a part of each respective metric 
registration by using {{SolrInfoBean.this.registerName(name)}}.
{quote}
Can you file a new Jira for this one and I'll grab it? Don't remember which 
class I used as template back then, but there may be other places to clean up 
too.

> 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



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

2019-08-16 Thread Andrzej Bialecki (JIRA)


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

Andrzej Bialecki  commented on SOLR-13700:
--

ad 1: yes, it's correct - the instances of these metrics are reused by all 
instances of respective plugins, so if the "old" instance was still processing 
an in-flight request it will still be counted, while the new instance may 
already handle new requests and use the same counters. It's perfectly ok.

ad 2: this seems like a confusion between referring to 
{{pkiAuthenticationPlugin}} (which stays unchanged) and 
{{authenticationPlugin}} (which may have changed). It's correct we don't need 
this here.

Also, I think the call to {{initializeMetrics}} should be done in each of 
{{initializeAuditloggerPlugin}} and {{initializeAuthenticationPlugin}}, then 
it's easier to avoid confusion because within the scope of these methods it's 
obvious that a new instance is created which needs to be initialized.

Also, I just spotted that both {{AuditLoggerPlugin.initializeMetrics}} and 
{{AuthenticationPlugin.initializeMetrics}} needlessly add metrics names to 
{{metricNames}} - this is already done as a part of each respective metric 
registration by using {{SolrInfoBean.this.registerName(name)}}.

> 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



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

2019-08-16 Thread JIRA


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

Jan Høydahl commented on SOLR-13700:


Good catch.

Re point 2, your patch deletes the wrong lines for auditloggerPlugin metrics. I 
agree that the initialization of PKI metrics in [lines 
877-879|https://github.com/apache/lucene-solr/blob/master/solr/core/src/java/org/apache/solr/core/CoreContainer.java#L877-L879]
 should be moved to L631, right after initializing PKI first (only) time.

> 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