kezhuw commented on code in PR #2192:
URL: https://github.com/apache/zookeeper/pull/2192#discussion_r1779466279
##########
zookeeper-server/src/main/java/org/apache/zookeeper/server/persistence/TxnLogToolkit.java:
##########
@@ -361,6 +371,40 @@ static String getFormattedTxnStr(Record txn) {
return txnData.toString();
}
+ private static Record deserializeTxn(Txn txn) throws IOException {
+ ByteBuffer bb = ByteBuffer.wrap(txn.getData());
Review Comment:
This is not used until the last statement. Better to move until usage.
##########
zookeeper-server/src/test/java/org/apache/zookeeper/server/persistence/TxnLogToolkitTest.java:
##########
@@ -95,27 +106,33 @@ public void testInitMissingFile() throws
FileNotFoundException, TxnLogToolkit.Tx
@Test
public void testMultiTxnDecode() throws IOException {
- //MultiTxn with four ops, and the first op error.
+ //MultiTxn with multi ops, the first op is error and the others are
normal
List<Txn> txns = new ArrayList<>();
- int type = -1;
- for (int i = 0; i < 4; i++) {
- ErrorTxn txn;
- if (i == 0) {
- txn = new ErrorTxn(KeeperException.Code.NONODE.intValue());
- } else {
- txn = new
ErrorTxn(KeeperException.Code.RUNTIMEINCONSISTENCY.intValue());
- }
+ Map<Record, Integer> records = new HashMap<>();
+ records.put(new ErrorTxn(KeeperException.Code.NONODE.intValue()),
ZooDefs.OpCode.error);
+ records.put(new
ErrorTxn(KeeperException.Code.RUNTIMEINCONSISTENCY.intValue()), -1);
+ records.put(new CreateTxn("/test",
"test-data".getBytes(StandardCharsets.UTF_8), ZooDefs.Ids.OPEN_ACL_UNSAFE,
true, 1), ZooDefs.OpCode.create);
+ records.put(new CreateContainerTxn("/test_container",
"test-data".getBytes(StandardCharsets.UTF_8), ZooDefs.Ids.OPEN_ACL_UNSAFE, 2),
ZooDefs.OpCode.createContainer);
+ records.put(new CreateTTLTxn("/test_container",
"test-data".getBytes(StandardCharsets.UTF_8), ZooDefs.Ids.OPEN_ACL_UNSAFE, 2,
20), ZooDefs.OpCode.createTTL);
+ records.put(new SetDataTxn("/test_set_data",
"test-data".getBytes(StandardCharsets.UTF_8), 4), ZooDefs.OpCode.setData);
+ records.put(new DeleteTxn("/test_delete"), ZooDefs.OpCode.delete);
+ records.put(new CheckVersionTxn("/test_check_version", 5),
ZooDefs.OpCode.check);
+
+ for (Map.Entry<Record, Integer> recordEntry : records.entrySet()) {
try (ByteArrayOutputStream baos = new ByteArrayOutputStream()) {
BinaryOutputArchive boa = BinaryOutputArchive.getArchive(baos);
- txn.serialize(boa, "request");
+ recordEntry.getKey().serialize(boa, "request");
ByteBuffer bb = ByteBuffer.wrap(baos.toByteArray());
- txns.add(new Txn(type, bb.array()));
+ txns.add(new Txn(recordEntry.getValue(), bb.array()));
}
}
Review Comment:
How about adding a helper method, say `newSubTxn(type, record)`, to
construct `Txn` ? This way we can add test case using `txns.add` sequentially.
##########
zookeeper-server/src/main/java/org/apache/zookeeper/server/persistence/TxnLogToolkit.java:
##########
@@ -361,6 +371,40 @@ static String getFormattedTxnStr(Record txn) {
return txnData.toString();
}
+ private static Record deserializeTxn(Txn txn) throws IOException {
Review Comment:
Rename to `deserializeSubTxn` ? I was confused until comparing it with
`SerializeUtils.deserializeTxn`.
##########
zookeeper-server/src/main/java/org/apache/zookeeper/server/persistence/TxnLogToolkit.java:
##########
@@ -361,6 +371,40 @@ static String getFormattedTxnStr(Record txn) {
return txnData.toString();
}
+ private static Record deserializeTxn(Txn txn) throws IOException {
+ ByteBuffer bb = ByteBuffer.wrap(txn.getData());
+ Record record;
+ switch (txn.getType()) {
+ case ZooDefs.OpCode.create:
+ case ZooDefs.OpCode.create2:
+ record = new CreateTxn();
+ break;
+ case ZooDefs.OpCode.createTTL:
+ record = new CreateTTLTxn();
+ break;
+ case ZooDefs.OpCode.createContainer:
+ record = new CreateContainerTxn();
+ break;
+ case ZooDefs.OpCode.delete:
+ case ZooDefs.OpCode.deleteContainer:
+ record = new DeleteTxn();
+ break;
+ case ZooDefs.OpCode.setData:
+ record = new SetDataTxn();
+ break;
+ case ZooDefs.OpCode.error:
+ record = new ErrorTxn();
+ break;
+ case ZooDefs.OpCode.check:
+ record = new CheckVersionTxn();
+ break;
+ default:
+ return null;
Review Comment:
We should also throw exception here, just like
`SerializeUtils.deserializeTxn`. Silence is not good in dumping.
##########
zookeeper-server/src/test/java/org/apache/zookeeper/server/persistence/TxnLogToolkitTest.java:
##########
@@ -95,27 +106,33 @@ public void testInitMissingFile() throws
FileNotFoundException, TxnLogToolkit.Tx
@Test
public void testMultiTxnDecode() throws IOException {
- //MultiTxn with four ops, and the first op error.
+ //MultiTxn with multi ops, the first op is error and the others are
normal
Review Comment:
```suggestion
// MultiTxn with multi ops including errors
```
--
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: notifications-unsubscr...@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
