Improve performance of queries against SYSTEM.SEGMENT table.#11008
Improve performance of queries against SYSTEM.SEGMENT table.#11008samarthjain merged 1 commit intoapache:masterfrom
Conversation
There was a problem hiding this comment.
should this be segmentMetadataInfo.values().stream().mapToInt(v -> v.size()).sum() ?
There was a problem hiding this comment.
Thanks for pointing that out. Fixed.
|
+1 after CI |
Size HashMap and HashSet appropriately. Perf analysis of the queries revealed that over 25% of the query time was spent in resizing HashMap and HashSet collections. Also, prevent the need to examine and authorize all resources when AllowAllAuthorizer is the configured authorizer.
samarthjain
force-pushed
the
console_perf
branch
from
5c23106 to
dcf2b6c
Compare
| throw new ISE("No authorizer found with name: [%s].", authenticationResult.getAuthorizerName()); | ||
| } | ||
|
|
||
| if (authorizer instanceof AllowAllAuthorizer) { |
There was a problem hiding this comment.
earlier this code will filter a resource if resourceActionGenerator.apply(resource) returns null but now it won't. does that difference matter?
There was a problem hiding this comment.
Yeah, looks like this isn't the right thing to do.
Also, the following loop needs to be executed because there could be duplicates in Iterable<ResourceAction. Will file an issue and a PR to revert this change.
There was a problem hiding this comment.
do you intend to get rid of the duplicates or keep them? From the description in #11012, it seems we want to de-dup the entries though that wasn't happening earlier.
4 participants
Fixes #11007 .
After hooking up Yourkit against the broker instance running the queries mentioned in the above issue, it turns out that a lot of time is being spent in resizing HashMap and HashSet that have been created with the default constructor. Also, a lot of CPU cycles are wasted unnecessarily looping through and authorizing resources even though the authorizer configured is AllowAllAuthorizer.
After making these changes, on my test cluster, I see that the query times went down from ~12seconds to ~7seconds.
This PR has: