keith-turner commented on code in PR #4258:
URL: https://github.com/apache/accumulo/pull/4258#discussion_r1492783538
##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletOperationId.java:
##########
@@ -33,14 +33,14 @@ public class TabletOperationId extends
AbstractId<TabletOperationId> {
public static String validate(String opid) {
var fields = opid.split(":");
- Preconditions.checkArgument(fields.length == 2, "Malformed operation id
%s", opid);
+ Preconditions.checkArgument(fields.length == 4, "Malformed operation id
%s", opid);
Review Comment:
Could use the following String method then, can leave the rest of the code
as is (like check for 2 and assume the field[1] is the entire fate id even if
it has ":")
```java
var fields = opid.split(":",2);
```
##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletOperationId.java:
##########
@@ -53,15 +53,15 @@ private TabletOperationId(String canonical) {
public TabletOperationType getType() {
var fields = canonical().split(":");
- Preconditions.checkState(fields.length == 2);
+ Preconditions.checkState(fields.length == 4);
Review Comment:
Could also use `split(":",2)` in this method.
##########
core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ZooReservation.java:
##########
@@ -29,15 +30,14 @@
public class ZooReservation {
- public static boolean attempt(ZooReaderWriter zk, String path, String
reservationID,
- String debugInfo) throws KeeperException, InterruptedException {
- if (reservationID.contains(":")) {
- throw new IllegalArgumentException();
- }
+ private static final String DELIMITER = "-";
+
+ public static boolean attempt(ZooReaderWriter zk, String path, FateId
fateId, String debugInfo)
+ throws KeeperException, InterruptedException {
while (true) {
try {
- zk.putPersistentData(path, (reservationID + ":" +
debugInfo).getBytes(UTF_8),
+ zk.putPersistentData(path, (fateId + DELIMITER +
debugInfo).getBytes(UTF_8),
Review Comment:
When using fate ids in persisted data, its a bit safer to call the
canonical() method vs toString() because its more well defined what the
intention is.
```suggestion
zk.putPersistentData(path, (fateId.canonical() + DELIMITER +
debugInfo).getBytes(UTF_8),
```
--
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]
