rpuch commented on code in PR #7769:
URL: https://github.com/apache/ignite-3/pull/7769#discussion_r2964386750
##########
modules/placement-driver/src/main/java/org/apache/ignite/internal/placementdriver/leases/LeaseBatchSerializer.java:
##########
@@ -538,6 +555,14 @@ private static boolean flagSet(int flags, int mask) {
return (flags & mask) != 0;
}
+ private static boolean holderIdAndProposedCandidateFitIn1ByteForRead(byte
protoVer, NodesDictionary dictionary) {
+ if (protoVer == PROTOCOL_V1) {
+ return dictionary.nameCount() <= MAX_NODES_FOR_COMPACT_MODE;
Review Comment:
Please add a comment explaining that in v1 we assumed that name and node
tables have the same size, hence we were only checking for name table size
##########
modules/placement-driver/src/test/java/org/apache/ignite/internal/placementdriver/leases/LeaseBatchSerializerTest.java:
##########
@@ -316,6 +318,63 @@ void v1CanBeDeserialized(String serializedString,
List<Lease> expectedLeases) {
);
}
+ @Test
+ void v1WithExactly8NamesAndMoreThan8NodeIdsCanBeDeserialized() throws
IOException {
+ byte[] bytes =
v1BytesWithExactly8NamesAndMoreThan8NodeIdsUsingCompactEncoding();
+
+ LeaseBatch restoredBatch = VersionedSerialization.fromBytes(bytes,
serializer);
+
+ Lease expectedLease = new Lease(
+ "node0",
+ new UUID(0, 1),
+ new HybridTimestamp(900, 0),
+ new HybridTimestamp(1000, 0),
+ true,
+ true,
+ "node1",
+ new TablePartitionId(1, 0)
+ );
+
+ assertThat(restoredBatch.leases(), containsInAnyOrder(expectedLease));
+ assertEquals(List.of("node1"),
restoredBatch.leases().stream().map(Lease::proposedCandidate).collect(toList()));
+ }
+
+ private static byte[]
v1BytesWithExactly8NamesAndMoreThan8NodeIdsUsingCompactEncoding() throws
IOException {
+ try (IgniteUnsafeDataOutput out = new IgniteUnsafeDataOutput(256)) {
Review Comment:
Please explain in a comment why here the binary representation is produced
'by hand' (probably because it was not possible to produce it with assertions
enabled?)
##########
modules/placement-driver/src/test/java/org/apache/ignite/internal/placementdriver/leases/LeaseBatchSerializerTest.java:
##########
@@ -316,6 +318,63 @@ void v1CanBeDeserialized(String serializedString,
List<Lease> expectedLeases) {
);
}
+ @Test
+ void v1WithExactly8NamesAndMoreThan8NodeIdsCanBeDeserialized() throws
IOException {
+ byte[] bytes =
v1BytesWithExactly8NamesAndMoreThan8NodeIdsUsingCompactEncoding();
+
+ LeaseBatch restoredBatch = VersionedSerialization.fromBytes(bytes,
serializer);
+
+ Lease expectedLease = new Lease(
+ "node0",
+ new UUID(0, 1),
+ new HybridTimestamp(900, 0),
+ new HybridTimestamp(1000, 0),
+ true,
+ true,
+ "node1",
+ new TablePartitionId(1, 0)
+ );
+
+ assertThat(restoredBatch.leases(), containsInAnyOrder(expectedLease));
+ assertEquals(List.of("node1"),
restoredBatch.leases().stream().map(Lease::proposedCandidate).collect(toList()));
+ }
+
+ private static byte[]
v1BytesWithExactly8NamesAndMoreThan8NodeIdsUsingCompactEncoding() throws
IOException {
+ try (IgniteUnsafeDataOutput out = new IgniteUnsafeDataOutput(256)) {
+ // Header written by VersionedSerializer.writeExternal():
MAGIC(0x43BEEF00) + protocolVersion(1).
+ out.writeInt(0x43BEEF01);
+
+ // Lease batch header.
+ out.writeVarInt(1000); // minExpirationTimePhysical
+ out.writeVarInt(0); // commonExpirationTimePhysicalDelta
+ out.writeVarInt(0); // commonExpirationTimeLogical
+
+ // Nodes dictionary: 8 names.
+ out.writeVarInt(8);
+ for (int i = 0; i < 8; i++) {
+ out.writeUTF("node" + i);
+ }
+
+ // Nodes dictionary: 9 node IDs, with names cycling over 8 entries.
+ out.writeVarInt(9);
+ for (int i = 0; i < 9; i++) {
+ out.writeUuid(new UUID(0, i + 1));
+ out.writeVarInt(i % 8);
+ }
+
+ // Table section with one object and one lease.
+ out.writeVarInt(1); // tableCount
+ out.writeVarInt(1); // objectId delta
+ out.writeVarInt(1); // leaseCount
+ out.write(0x07); // accepted + prolongable + hasProposedCandidate
+ out.writeVarInt(8); // compact nodes info: holder=0,
proposedCandidate=1
+ out.writeVarInt(100); // period (expirationPhysical -
startPhysical)
+ out.writeVarInt(0); // startLogical
+
+ return out.array();
+ }
+ }
+
Review Comment:
Please add a test verifying that object serialized with v2 can be
deserialized
--
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]
