IGNITE-28013 Fix stale leaseholder ID handling in LeaseUpdater and lease batch serialization#7769
Conversation
…ase batch serialization
c05b98d to
6941fb7
Compare
| for (LogicalNode node : logicalTopologySnap0.nodes()) { | ||
| if (node.id().equals(nodeId)) { | ||
| return true; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
probably, this method and nodeByConsistentId should be refactored
| if (hasStaleLeaseholderId) { | ||
| LOG.info("Leaseholder has left the logical topology, creating a new lease [groupId={}, lease={}, candidate={}]", | ||
| grpId, lease, candidate); | ||
|
|
||
| if (candidate == null) { | ||
| logGroupWithoutCandidateOnce(grpId, true, stableAssignments, pendingAssignments); | ||
| continue; | ||
| } | ||
|
|
||
| Lease newLease = writeNewLease(grpId, candidate, renewedLeases); | ||
|
|
||
| boolean force = Objects.equals(lease.getLeaseholder(), candidate.name()); | ||
|
|
||
| toBeNegotiated.put(grpId, new LeaseAgreement(newLease, force)); | ||
|
|
||
| continue; | ||
| } | ||
|
|
There was a problem hiding this comment.
actually, all you need is to exclude the outcome when tryToFindCandidateAmongAssignments returns stale candidate, that would be much more simple
There was a problem hiding this comment.
I can, but that leaves a known-stale lease published longer than necessary.
…the issue we change serialization. Moreover, added test to check that non-expired leases are not overriden.
...er/src/main/java/org/apache/ignite/internal/placementdriver/leases/LeaseBatchSerializer.java
Show resolved
Hide resolved
|
|
||
| private static boolean holderIdAndProposedCandidateFitIn1ByteForRead(byte protoVer, NodesDictionary dictionary) { | ||
| if (protoVer == PROTOCOL_V1) { | ||
| return dictionary.nameCount() <= MAX_NODES_FOR_COMPACT_MODE; |
There was a problem hiding this 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
| } | ||
|
|
||
| private static byte[] v1BytesWithExactly8NamesAndMoreThan8NodeIdsUsingCompactEncoding() throws IOException { | ||
| try (IgniteUnsafeDataOutput out = new IgniteUnsafeDataOutput(256)) { |
There was a problem hiding this 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?)
| return out.array(); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Please add a test verifying that object serialized with v2 can be deserialized
https://issues.apache.org/jira/browse/IGNITE-28013