This is an automated email from the ASF dual-hosted git repository.
sanpwc pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git
The following commit(s) were added to refs/heads/main by this push:
new 6d9123eb6e IGNITE-23229 Fixed
StopLeaseProlongationMessageResponse#deniedLeaseExpirationTime may unbox a null
value and cause NPE (#4425)
6d9123eb6e is described below
commit 6d9123eb6e683852c16e057c71e2fb353f3334a0
Author: Denis Chudov <[email protected]>
AuthorDate: Thu Sep 19 19:34:07 2024 +0300
IGNITE-23229 Fixed
StopLeaseProlongationMessageResponse#deniedLeaseExpirationTime may unbox a null
value and cause NPE (#4425)
---
.../message/StopLeaseProlongationMessageResponse.java | 3 +--
.../ignite/internal/placementdriver/LeaseUpdater.java | 5 ++++-
.../ignite/internal/replicator/ReplicaManager.java | 16 ++++++++++------
3 files changed, 15 insertions(+), 9 deletions(-)
diff --git
a/modules/placement-driver-api/src/main/java/org/apache/ignite/internal/placementdriver/message/StopLeaseProlongationMessageResponse.java
b/modules/placement-driver-api/src/main/java/org/apache/ignite/internal/placementdriver/message/StopLeaseProlongationMessageResponse.java
index a1aa8eece1..90a3b4445e 100644
---
a/modules/placement-driver-api/src/main/java/org/apache/ignite/internal/placementdriver/message/StopLeaseProlongationMessageResponse.java
+++
b/modules/placement-driver-api/src/main/java/org/apache/ignite/internal/placementdriver/message/StopLeaseProlongationMessageResponse.java
@@ -33,8 +33,7 @@ public interface StopLeaseProlongationMessageResponse extends
PlacementDriverMes
*
* @return Lease expiration time of denied lease.
*/
- @Nullable
- Long deniedLeaseExpirationTimeLong();
+ long deniedLeaseExpirationTimeLong();
@Nullable
default HybridTimestamp deniedLeaseExpirationTime() {
diff --git
a/modules/placement-driver/src/main/java/org/apache/ignite/internal/placementdriver/LeaseUpdater.java
b/modules/placement-driver/src/main/java/org/apache/ignite/internal/placementdriver/LeaseUpdater.java
index 1c76765a9c..ef26725948 100644
---
a/modules/placement-driver/src/main/java/org/apache/ignite/internal/placementdriver/LeaseUpdater.java
+++
b/modules/placement-driver/src/main/java/org/apache/ignite/internal/placementdriver/LeaseUpdater.java
@@ -19,6 +19,7 @@ package org.apache.ignite.internal.placementdriver;
import static java.util.Objects.hash;
import static java.util.Objects.requireNonNullElse;
+import static
org.apache.ignite.internal.hlc.HybridTimestamp.NULL_HYBRID_TIMESTAMP;
import static org.apache.ignite.internal.lang.IgniteStringFormatter.format;
import static org.apache.ignite.internal.metastorage.dsl.Conditions.notExists;
import static org.apache.ignite.internal.metastorage.dsl.Conditions.or;
@@ -733,7 +734,9 @@ public class LeaseUpdater {
}
if (correlationId != null) {
- Long deniedLeaseExpTimeLong = deniedLeaseExpTime
== null ? null : deniedLeaseExpTime.longValue();
+ long deniedLeaseExpTimeLong = deniedLeaseExpTime
== null
+ ? NULL_HYBRID_TIMESTAMP
+ : deniedLeaseExpTime.longValue();
StopLeaseProlongationMessageResponse response =
PLACEMENT_DRIVER_MESSAGES_FACTORY
.stopLeaseProlongationMessageResponse()
diff --git
a/modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java
b/modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java
index d69add472a..472fd34ecf 100644
---
a/modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java
+++
b/modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java
@@ -45,7 +45,6 @@ import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
-import java.util.Objects;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionException;
@@ -605,21 +604,26 @@ public class ReplicaManager extends
AbstractEventProducer<LocalReplicaEvent, Loc
}
}
+ // We send StopLeaseProlongationMessage on every node of
placement driver group, so there should be all nulls or
+ // just one non-null, possible outcomes:
+ // - it wasn't successfully handled anywhere (lease updater
thread made successful ms.invoke, and SLPM handlers failed
+ // to do ms.invoke)
+ // - it was successfully handled on one node of PD group, in
this case we get one non-null
+ // - it was successfully handled on some node, but message
handling was delayed on some other node and it already got lease
+ // update from MS where this lease was denied, in this case
it also returns null (slightly other case than
+ // failed ms.invoke but same outcome)
return allOf(futs)
.thenCompose(unused -> {
NetworkMessage response = futs.stream()
.map(CompletableFuture::join)
- .filter(Objects::nonNull)
+ .filter(resp -> resp instanceof
StopLeaseProlongationMessageResponse
+ &&
((StopLeaseProlongationMessageResponse) resp).deniedLeaseExpirationTime() !=
null)
.findAny()
.orElse(null);
if (response == null) {
return stopLeaseProlongation(groupId,
redirectNodeId, endTime);
} else {
- assert response instanceof
StopLeaseProlongationMessageResponse : format(
- "Unexpected response type [class={}, "
- + "response={}].",
response.getClass(), response);
-
return
completedFuture(((StopLeaseProlongationMessageResponse)
response).deniedLeaseExpirationTime());
}
});
