ctubbsii commented on code in PR #5262:
URL: https://github.com/apache/accumulo/pull/5262#discussion_r1915815637
##########
core/src/main/java/org/apache/accumulo/core/fate/zookeeper/FateLock.java:
##########
@@ -68,16 +72,34 @@ public FateLock(ZooReaderWriter zrw, FateLockPath path) {
this.path = requireNonNull(path);
}
+ public static class FateLockNode {
+ public final long sequence;
+ public final String lockData;
+
+ private FateLockNode(String nodeName) {
+ int len = nodeName.length();
+ Preconditions.checkArgument(nodeName.startsWith(PREFIX) &&
nodeName.charAt(len - 11) == '#',
+ "Illegal node name %s", nodeName);
+ sequence = Long.parseLong(nodeName.substring(len - 10));
+ lockData = nodeName.substring(PREFIX.length(), len - 11);
+ }
+ }
+
+ // TODO change data arg from byte[] to String.. in the rest of the code its
always a String.
@Override
public long addEntry(byte[] data) {
+
+ String dataString = new String(data, UTF_8);
+ Preconditions.checkState(!dataString.contains("#"));
+
String newPath;
try {
while (true) {
try {
- newPath = zoo.putPersistentSequential(path + "/" + PREFIX, data);
+ newPath = zoo.putPersistentSequential(path + "/" + PREFIX +
dataString + "#", data);
Review Comment:
```suggestion
newPath = zoo.putPersistentSequential(path + "/" + PREFIX +
dataString + "#", new byte[0]);
```
##########
core/src/main/java/org/apache/accumulo/core/fate/AdminUtil.java:
##########
@@ -285,19 +286,19 @@ private void findLocks(ZooSession zk, final
ServiceLockPath lockPath,
List<String> lockedIds = zr.getChildren(lockPath.toString());
for (String id : lockedIds) {
-
try {
-
FateLockPath fLockPath = FateLock.path(lockPath + "/" + id);
- List<String> lockNodes =
- FateLock.validateAndSort(fLockPath,
zr.getChildren(fLockPath.toString()));
+ List<FateLock.FateLockNode> lockNodes =
+ FateLock.validateAndWarn(fLockPath,
zr.getChildren(fLockPath.toString()));
+
+ lockNodes.sort(Comparator.comparingLong(ln -> ln.sequence));
Review Comment:
Should the return type be a sorted set instead of a list that you sort later?
##########
core/src/main/java/org/apache/accumulo/core/fate/zookeeper/DistributedReadWriteLock.java:
##########
@@ -251,19 +253,28 @@ public DistributedReadWriteLock(QueueLock qlock, byte[]
data) {
}
public static DistributedLock recoverLock(QueueLock qlock, byte[] data) {
- SortedMap<Long,byte[]> entries = qlock.getEarlierEntries(Long.MAX_VALUE);
- for (Entry<Long,byte[]> entry : entries.entrySet()) {
- ParsedLock parsed = new ParsedLock(entry.getValue());
- if (Arrays.equals(data, parsed.getUserData())) {
+ SortedMap<Long,byte[]> entries = qlock.getEntries((seq, lockData) -> {
+ ParsedLock parsed = new ParsedLock(lockData);
+ return Arrays.equals(data, parsed.getUserData());
+ });
+
+ switch (entries.size()) {
+ case 0:
+ return null;
+ case 1:
+ var entry = entries.entrySet().iterator().next();
+ ParsedLock parsed = new ParsedLock(entry.getValue());
switch (parsed.getType()) {
case READ:
return new ReadLock(qlock, parsed.getUserData(), entry.getKey());
case WRITE:
return new WriteLock(qlock, parsed.getUserData(), entry.getKey());
+ default:
+ throw new IllegalStateException("Uknown lock type " +
parsed.getType());
Review Comment:
```suggestion
throw new IllegalStateException("Unknown lock type " +
parsed.getType());
```
--
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]
