vldpyatkov commented on code in PR #2453:
URL: https://github.com/apache/ignite-3/pull/2453#discussion_r1296817329
##########
modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java:
##########
@@ -913,6 +914,29 @@ public static void inBusyLock(IgniteSpinBusyLock busyLock,
Runnable fn) {
}
}
+ /**
+ * Method that runs the provided {@code fn} in {@code busyLock}.
+ *
+ * @param <T> Type of returned value from {@code fn}.
+ * @param busyLock Component's busy lock.
+ * @param fn Function to run.
+ * @return Future returned from the {@code fn}, or future with the {@link
NodeStoppingException} if
+ * {@link IgniteSpinBusyLock#enterBusy()} failed or with runtime
exception/error while executing the {@code fn}.
+ */
+ public static <T> CompletableFuture<T> inBusyLockAsync(IgniteSpinBusyLock
busyLock, Supplier<CompletableFuture<T>> fn) {
Review Comment:
I seriously doubt this method because it looks very unclear in the code and
provokes closure creation. A code with a plain checking lock looks easier to
read.
Moreover, I also do not advise abusing the methods (inBusyLock) above for
the same reason.
But it is your choose.
##########
modules/placement-driver/src/main/java/org/apache/ignite/internal/placementdriver/leases/LeaseTracker.java:
##########
@@ -154,60 +147,59 @@ public void stopTrack() {
* @param grpId Replication group id.
* @return A lease is associated with the group.
*/
- public @NotNull Lease getLease(ReplicationGroupId grpId) {
+ public Lease getLease(ReplicationGroupId grpId) {
+ Leases leases = this.leases;
+
assert leases != null : "Leases not initialized, probably the local
placement driver actor hasn't started lease tracking.";
- return leases.get1().getOrDefault(grpId, EMPTY_LEASE);
+ return leases.leaseByGroupId().getOrDefault(grpId, EMPTY_LEASE);
}
- /**
- * Returns collection of leases, ordered by replication group.
- *
- * @return Collection of leases.
- */
- public IgniteBiTuple<Map<ReplicationGroupId, Lease>, byte[]>
leasesCurrent() {
+ /** Returns collection of leases, ordered by replication group. */
+ public Leases leasesCurrent() {
return leases;
}
- /**
- * Listen lease holder updates.
- */
+ /** Listen lease holder updates. */
private class UpdateListener implements WatchListener {
@Override
public CompletableFuture<Void> onUpdate(WatchEvent event) {
- for (EntryEvent entry : event.entryEvents()) {
- Entry msEntry = entry.newEntry();
+ return inBusyLockAsync(busyLock, () -> {
Review Comment:
Is the lock really needed here?
--
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]
