vldpyatkov commented on code in PR #2141:
URL: https://github.com/apache/ignite-3/pull/2141#discussion_r1250788223
##########
modules/placement-driver/src/integrationTest/java/org/apache/ignite/internal/placementdriver/MultiActorPlacementDriverTest.java:
##########
@@ -91,7 +96,7 @@
/**
* There are tests of muti-nodes for placement driver.
*/
-@ExtendWith(ConfigurationExtension.class)
+@ExtendWith({ConfigurationExtension.class, WorkDirectoryExtension.class})
Review Comment:
IgniteAbstractTest already has WorkDirectoryExtension.
You can use workDir variable to access it.
##########
modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java:
##########
@@ -1087,4 +1088,56 @@ public static boolean startsWith(byte[] key, byte[]
prefix) {
return key.length >= prefix.length
&& Arrays.equals(key, 0, prefix.length, prefix, 0,
prefix.length);
}
+
+ /**
+ * Serializes collection to bytes.
+ *
+ * @param collection Collection.
+ * @param transform Tranform function for the collection element.
+ * @return Byte array.
+ */
+ public static <T> byte[] collectionToBytes(Collection<T> collection,
Function<T, byte[]> transform) {
+ int bytesObjects = 0;
+ List<byte[]> objects = new ArrayList<>();
+
+ for (T o : collection) {
+ byte[] b = transform.apply(o);
+ objects.add(b);
+ bytesObjects += b.length;
+ }
+
+ bytesObjects += Integer.BYTES * (objects.size() + 1);
Review Comment:
One Integer.BYTES should be enough
##########
modules/placement-driver/src/main/java/org/apache/ignite/internal/placementdriver/LeaseUpdater.java:
##########
@@ -207,17 +215,25 @@ public void deactivate() {
* @return Future completes true when the lease will not prolong in the
future, false otherwise.
*/
public CompletableFuture<Boolean> denyLease(ReplicationGroupId grpId,
Lease lease) {
- var leaseKey = ByteArray.fromString(PLACEMENTDRIVER_PREFIX + grpId);
-
- byte[] leaseRaw = lease.bytes();
-
Lease deniedLease = lease.denyLease();
leaseNegotiator.onLeaseRemoved(grpId);
+ Collection<Lease> leases = leaseTracker.leasesCurrent();
+ List<Lease> renewedLeases = new ArrayList<>();
+ for (Lease ls : leases) {
+ if (ls.replicationGroupId().equals(grpId)) {
+ renewedLeases.add(deniedLease);
+ } else {
+ renewedLeases.add(ls);
+ }
+ }
+
+ var key = PLACEMENTDRIVER_LEASES_KEY;
+
return msManager.invoke(
- value(leaseKey).eq(leaseRaw),
- put(leaseKey, deniedLease.bytes()),
+ or(notExists(key), value(key).eq(new
LeaseBatch(leaseTracker.leasesCurrent()).bytes())),
+ put(key, new LeaseBatch(renewedLeases).bytes()),
Review Comment:
Here you serialize two times, each time when .bytes() is invoked.
Do this only once.
##########
modules/placement-driver/src/main/java/org/apache/ignite/internal/placementdriver/LeaseUpdater.java:
##########
@@ -305,14 +328,33 @@ public void run() {
// leaseholders at all.
if (isLeaseOutdated(lease)) {
// New lease is granting.
- writeNewLeaseInMetaStorage(grpId, lease,
candidate);
+ writeNewLeaseInMetaStorage(grpId, lease,
candidate, renewedLeases, toBeNegotiated);
} else if (lease.isProlongable() &&
candidate.name().equals(lease.getLeaseholder())) {
// Old lease is renewing.
- prolongLeaseInMetaStorage(grpId, lease);
+ prolongLeaseInMetaStorage(grpId, lease,
renewedLeases);
}
}
}
+ byte[] renewedValue = new
LeaseBatch(renewedLeases.values()).bytes();
+
+ var key = PLACEMENTDRIVER_LEASES_KEY;
+
+ msManager.invoke(
+ or(notExists(key), value(key).eq(new
LeaseBatch(leaseTracker.leasesCurrent()).bytes())),
Review Comment:
May be better to provide API to avoid extra serialization?
##########
modules/placement-driver/src/main/java/org/apache/ignite/internal/placementdriver/LeaseUpdater.java:
##########
@@ -328,28 +370,17 @@ public void run() {
* @param lease Old lease to apply CAS in Meta storage.
* @param candidate Lease candidate.
*/
- private void writeNewLeaseInMetaStorage(ReplicationGroupId grpId,
Lease lease, ClusterNode candidate) {
- var leaseKey = ByteArray.fromString(PLACEMENTDRIVER_PREFIX +
grpId);
-
+ private void writeNewLeaseInMetaStorage(ReplicationGroupId grpId,
Lease lease, ClusterNode candidate,
Review Comment:
The method is not writing something to MS anymore. I think, it should be
renamed with correction of java doc.
--
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]
