ctubbsii commented on code in PR #2707:
URL: https://github.com/apache/accumulo/pull/2707#discussion_r874753972
##########
server/base/src/main/java/org/apache/accumulo/server/security/handler/ZKSecurityTool.java:
##########
@@ -114,16 +119,34 @@ public static byte[] createPass(byte[] password) throws
AccumuloException {
return cryptHash.getBytes(UTF_8);
}
+ private static final Cache<String,String> CRYPT_PASSWORD_CACHE =
Caffeine.newBuilder()
+ .scheduler(Scheduler.systemScheduler()).expireAfterWrite(3,
TimeUnit.SECONDS).build();
Review Comment:
> > For this to be of benefit, we'd have to be creating a lot of connections
pretty quickly. I'm not sure how realistic that would be.
>
> I think that is what is described in #2700 as part of a test. I could see
real work implications, typically on application startup where you have a
thundering herd type situation.
I suppose. I'm just not sure how well the observations of lock contention or
brief periods of high CPU usage translate to end user experience, or overall
user application performance. This isn't much complexity, but in general, I'm
resistant to specialized code in our application that tries to work around
performance limitations in user hardware/external libraries, just because of
separation of responsibilities. I'm also resistant to workarounds that cater to
specific deployment use cases when better deployment decisions (like choosing a
better JCE library or deploying using hardware with crypto extensions) could
also address the problem. This hits on both those points, and it's not clear
it's even noticeable to users. But, the workaround is simple enough, so maybe
it's okay.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
