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