[WFCORE-3415] Faster resource tree size calculation for getMBeanCount() #2930
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bstansberry
force-pushed
the
child-count
branch
from
a5c9654
to
382a45d
Compare
bstansberry
changed the title
bstansberry
force-pushed
the
child-count
branch
2 times, most recently
from
50246ed
to
7233807
Compare
|
Full integration - Windows Build 4556 outcome was FAILURE using a merge of 7233807 Failed tests |
|
@fjuma Another org.jboss.as.test.integration.jsf.doctype.DoctypeDeclTestCase.testDoctypeDeclAllowed failure. Can you ping me about this to discuss details? |
kabir
approved these changes
Nov 13, 2017
bstansberry
force-pushed
the
child-count
branch
from
7233807
to
51ff5fb
Compare
|
Full integration - Windows Build 4564 outcome was FAILURE using a merge of 51ff5fb |
The biggest thing this does is eliminate RBAC checks as a factor in determining whether a resource appears in the count. Which means, in the case of non-addressable resources the total number of mbeans a user could "see", for example via MBeanServerConnection.queryNames, could be less than the number returned by a simultaneous call to getMBeanCount(). The other big thing this does is avoiding doing two complete counts for the jboss.as and jboss.as.expr JMX domains. There should be no difference in counts between those so if both are installed it just counts one and doubles the result.
bstansberry
force-pushed
the
child-count
branch
from
51ff5fb
to
d9f38c6
Compare
|
Core - Full Integration Build 6131 outcome was FAILURE using a merge of d9f38c6 Failed tests |
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The biggest thing this does is eliminate RBAC checks as a factor in determining whether a resource appears in the count. Which means, in the case of non-addressable resources the total number of mbeans a user could "see", for example via MBeanServerConnection.queryNames, could be less than the number returned by a simultaneous call to getMBeanCount().
The other big thing this does is avoiding doing two complete counts for the jboss.as and jboss.as.expr JMX domains. There should be no difference in counts between those so if both are installed it just counts one and doubles the result.
I have a variant of this that retains the old getMBeanCount logic plus adds a simpler optimization of what's already there, and then adds this, and then times all 3, logging the times and the mbean counts. Against current wildfly master, standalone-full-ha.xml, it reports, for three tests:
The call times are roughly 100 times less, with the first call using the existing method taking 753ms! See https://issues.jboss.org/browse/WFLY-9408 for an example of a user-reported problem due to this. The approach used in this PR was also about 7ms faster than the alternative optimization approach.
If I map the user I'm connecting as to the Monitor RBAC role and then turn on RBAC, you can see the mbean count that results from not using RBAC to trim mbeans the user won't be able to address from the count:
Looking at the JMX specification (https://docs.oracle.com/javase/8/docs/technotes/guides/jmx/JMX_1_4_specification.pdf) I see nothing directly on the topic of the effect of any security checks on the result of getMBeanCount(). Section 12.1.2.4 "Permission Checking for Queries" clearly states that permission checks should remove mbeans that should not be visible according to the permission scheme from the query result, so definitely queries returning a subset of the entire mbean universe is contemplated. Now, that section is about security manager permissions, not our RBAC scheme, but it's the most on topic thing I see.