demery-pivotal commented on a change in pull request #7206:
URL: https://github.com/apache/geode/pull/7206#discussion_r771593290
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientUserAuths.java
##########
@@ -97,21 +93,20 @@ public long putSubject(@NotNull Subject subject, long
existingUniqueId) {
return newId;
}
- public ClientUserAuths(int clientProxyHashcode) {
- m_seed = clientProxyHashcode;
- uniqueIdGenerator = new Random(m_seed + System.currentTimeMillis());
- m_firstId = uniqueIdGenerator.nextLong();
+ public ClientUserAuths(SubjectIdGenerator idGenerator) {
+ this.idGenerator = idGenerator;
}
- synchronized long getNextID() {
- final long uniqueId = uniqueIdGenerator.nextLong();
- if (uniqueId == m_firstId) {
- uniqueIdGenerator = new Random(m_seed + System.currentTimeMillis());
- m_firstId = uniqueIdGenerator.nextLong();
+ private synchronized long getNextID() {
+ long uniqueId = idGenerator.generateId();
+ if (uniqueId == 0) {
+ uniqueId = idGenerator.generateId();
+ }
+ if (uniqueId == -1) {
Review comment:
I think I'm going to leave this as `-1` (but see below for an
alternative). The reason is that there's already a weird, subtle coupling
between the `-1` here (where it means "IDs exhausted") and the `-1` in
`putSubject()` (where it means "please generate an ID").
I say "weird" because, though `-1` has a different meaning in each place,
both places must use the same value. That's weird.
If I change this one to `IDS_EXHAUSTED`, that hides the fact that we must
use `-1` in both places… which makes it no less weird, but even more subtle.
I'm not happy with the weirdness, the subtlety, or the coupling. But given
that there's coupling and it's weird, I'm reluctant make it more subtle.
I considered making `generateId()` return an `OptionalLong`, which would
eliminate the need for the generator to treat `-1` as special in any way. I
decided not to… out of a perhaps unfounded fear of allocating the
`OptionalLong` on the heap. Should I reconsider that?
--
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]