Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configurable COLLISION_BUFFER_POWER #4707

Merged
merged 2 commits into from
Feb 23, 2021
Merged

Conversation

jbescos
Copy link
Member

@jbescos jbescos commented Jan 29, 2021

Relates to #4152

Running test with 100000 requests and keeping default value (8):
image

Running test with 100000 requests and COLLISION_BUFFER_POWER = 3:
image

@jbescos jbescos force-pushed the aggregated branch 2 times, most recently from 2ec9593 to 4d2eb3f Compare February 4, 2021 09:54
@jbescos jbescos force-pushed the aggregated branch 2 times, most recently from ec9b6c8 to c43095b Compare February 18, 2021 10:35
Signed-off-by: Jorge Bescos Gascon <[email protected]>
};
int collisionBuffePower = DEFAULT_COLLISION_BUFFER_POWER;
try {
collisionBuffePower = action.run();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually I'm not sure this will work in favor of AccessController.doPrivileged(action); because purpose of the AccessController is to provide to privileged action those privileges which previously that action did not have. So, AccessControler shall supply those

 // Needed by TimeWindowStatisticsImplTest
  permission java.util.PropertyPermission "jersey.config.server.monitoring.collision.buffer.power", "read,write";

you are providing for test purposes to the privileged action of getting property. While securityManager is on for sure.
And running action just by run() method will just invoke it as usual method without any additional privileges.
And SecurityException might not be caught when using AccessController because in that case user can supply proper privileges to be provided to action and thus SecurityException won't be thrown. On the other hand when you just run the action and exception occurs there is no way for privileges to be supplied to it so it will always fail and application will always use default collision buffer.

Copy link
Contributor

@senivam senivam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see note in the source code

Signed-off-by: Jorge Bescos Gascon <[email protected]>
Copy link
Contributor

@senivam senivam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@senivam senivam merged commit a87ae6a into eclipse-ee4j:master Feb 23, 2021
@senivam senivam added this to the 2.34 milestone Feb 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants