From 2c43e932ba9450375d6df77a9dd2b3e3582f32f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Avard=20Ottestad?= Date: Sat, 5 Aug 2023 21:40:07 +0200 Subject: [PATCH 1/3] GH-4711 ParallelTaskBase waits for up to 100ms for the underlying task to finish after it was cancelled and interrupted --- .../concurrent/ParallelTaskBase.java | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tools/federation/src/main/java/org/eclipse/rdf4j/federated/evaluation/concurrent/ParallelTaskBase.java b/tools/federation/src/main/java/org/eclipse/rdf4j/federated/evaluation/concurrent/ParallelTaskBase.java index fe1a565c3cc..d49de629cb3 100644 --- a/tools/federation/src/main/java/org/eclipse/rdf4j/federated/evaluation/concurrent/ParallelTaskBase.java +++ b/tools/federation/src/main/java/org/eclipse/rdf4j/federated/evaluation/concurrent/ParallelTaskBase.java @@ -107,7 +107,23 @@ public void close() { logger.debug("Attempting to cancel task {}", this); boolean successfullyCanceled = scheduledFuture.cancel(true); if (!successfullyCanceled) { - logger.debug("Task {} could not be cancelled properly.", this); + logger.debug("Task {} could not be cancelled properly. Maybe it has already completed.", + this); + } + + int timeout = 100; + for (int i = 0; i < timeout && !scheduledFuture.isDone(); i++) { + try { + Thread.sleep(1); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + break; + } + } + + if (!scheduledFuture.isDone()) { + logger.error("Timeout while waiting for task {} to terminate after it was cancelled.", + this); } } } From ab04ad71f7f39768a0d50e2db68593a5d117cf8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Avard=20Ottestad?= Date: Sat, 5 Aug 2023 21:40:20 +0200 Subject: [PATCH 2/3] GH-4711 improve interruption handling and improve closing of iterations --- .../rdf4j/common/iteration/Iterations.java | 36 ++++++++++--------- .../concurrent/ParallelServiceExecutor.java | 4 ++- .../CloseDependentConnectionIteration.java | 6 ++++ .../iterator/ConsumingIteration.java | 13 ++++++- .../join/ParallelServiceJoinTask.java | 4 ++- .../federated/optimizer/SourceSelection.java | 4 +-- 6 files changed, 46 insertions(+), 21 deletions(-) diff --git a/core/common/iterator/src/main/java/org/eclipse/rdf4j/common/iteration/Iterations.java b/core/common/iterator/src/main/java/org/eclipse/rdf4j/common/iteration/Iterations.java index f068e2235ee..14d5ea67611 100644 --- a/core/common/iterator/src/main/java/org/eclipse/rdf4j/common/iteration/Iterations.java +++ b/core/common/iterator/src/main/java/org/eclipse/rdf4j/common/iteration/Iterations.java @@ -50,11 +50,13 @@ public static List asList(Iteration * @return a List containing all elements obtained from the specified iteration. */ public static List asList(CloseableIteration iter) throws X { - // stream.collect is slightly slower than addAll for lists - List list = new ArrayList<>(); + try (iter) { + // stream.collect is slightly slower than addAll for lists + List list = new ArrayList<>(); - // addAll closes the iteration - return addAll(iter, list); + // addAll closes the iteration + return addAll(iter, list); + } } /** @@ -115,15 +117,12 @@ public static > C addAll(Iterati */ public static > C addAll(CloseableIteration iter, C collection) throws X { - try { + try (iter) { while (iter.hasNext()) { collection.add(iter.next()); } - } finally { - closeCloseable(iter); + return collection; } - - return collection; } /** @@ -215,9 +214,11 @@ public static String toString(Iteration iteration, S * @return A String representation of the objects provided by the supplied iteration. */ public static String toString(CloseableIteration iteration, String separator) throws X { - StringBuilder sb = new StringBuilder(); - toString(iteration, separator, sb); - return sb.toString(); + try (iteration) { + StringBuilder sb = new StringBuilder(); + toString(iteration, separator, sb); + return sb.toString(); + } } /** @@ -253,11 +254,13 @@ public static void toString(Iteration iteration, Str public static void toString(CloseableIteration iteration, String separator, StringBuilder sb) throws X { - while (iteration.hasNext()) { - sb.append(iteration.next()); + try (iteration) { + while (iteration.hasNext()) { + sb.append(iteration.next()); - if (iteration.hasNext()) { - sb.append(separator); + if (iteration.hasNext()) { + sb.append(separator); + } } } @@ -295,4 +298,5 @@ public static Set asSet(CloseableIteration performTaskIn // Note: in order two avoid deadlocks we consume the SERVICE result. // This is basically required to avoid processing background tuple // request (i.e. HTTP slots) in the correct order. - return new CollectionIteration<>(Iterations.asList(strategy.evaluate(service.getService(), bindings))); + try (var evaluate = strategy.evaluate(service.getService(), bindings)) { + return new CollectionIteration<>(Iterations.asList(evaluate)); + } } @Override diff --git a/tools/federation/src/main/java/org/eclipse/rdf4j/federated/evaluation/iterator/CloseDependentConnectionIteration.java b/tools/federation/src/main/java/org/eclipse/rdf4j/federated/evaluation/iterator/CloseDependentConnectionIteration.java index 6398ad33a8d..2014c88d15f 100644 --- a/tools/federation/src/main/java/org/eclipse/rdf4j/federated/evaluation/iterator/CloseDependentConnectionIteration.java +++ b/tools/federation/src/main/java/org/eclipse/rdf4j/federated/evaluation/iterator/CloseDependentConnectionIteration.java @@ -38,6 +38,12 @@ public CloseDependentConnectionIteration(CloseableIteration performTaskIn // Note: in order two avoid deadlocks we consume the SERVICE result. // This is basically required to avoid processing background tuple // request (i.e. HTTP slots) in the correct order. - return new CollectionIteration<>(Iterations.asList(strategy.evaluateService(expr, bindings))); + try (var iter = strategy.evaluateService(expr, bindings)) { + return new CollectionIteration<>(Iterations.asList(iter)); + } } @Override diff --git a/tools/federation/src/main/java/org/eclipse/rdf4j/federated/optimizer/SourceSelection.java b/tools/federation/src/main/java/org/eclipse/rdf4j/federated/optimizer/SourceSelection.java index ab40d3d2009..1b6a3cf1a3e 100644 --- a/tools/federation/src/main/java/org/eclipse/rdf4j/federated/optimizer/SourceSelection.java +++ b/tools/federation/src/main/java/org/eclipse/rdf4j/federated/optimizer/SourceSelection.java @@ -321,8 +321,8 @@ public ParallelCheckTask(Endpoint endpoint, StatementPattern stmt, QueryInfo que protected CloseableIteration performTaskInternal() throws Exception { try { TripleSource t = endpoint.getTripleSource(); - boolean hasResults; - hasResults = t.hasStatements(stmt, EmptyBindingSet.getInstance(), queryInfo, queryInfo.getDataset()); + boolean hasResults = t.hasStatements(stmt, EmptyBindingSet.getInstance(), queryInfo, + queryInfo.getDataset()); SourceSelection sourceSelection = control.sourceSelection; sourceSelection.cache.updateInformation(new SubQuery(stmt, queryInfo.getDataset()), endpoint, From c8260326358b967998d15805f7227d4142a34d60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Avard=20Ottestad?= Date: Sat, 5 Aug 2023 22:29:33 +0200 Subject: [PATCH 3/3] GH-4714 be more defensive in case a bug results in value being null even though it was expected --- .../sail/shacl/results/ValidationResult.java | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/results/ValidationResult.java b/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/results/ValidationResult.java index d78ceb99df1..9c0fb603f3b 100644 --- a/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/results/ValidationResult.java +++ b/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/results/ValidationResult.java @@ -38,6 +38,8 @@ import org.eclipse.rdf4j.sail.shacl.ast.constraintcomponents.ConstraintComponent; import org.eclipse.rdf4j.sail.shacl.ast.constraintcomponents.SparqlConstraintComponent; import org.eclipse.rdf4j.sail.shacl.ast.paths.Path; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * The ValidationResult represents the results from a SHACL validation in an easy-to-use Java API. @@ -48,6 +50,8 @@ @Deprecated public class ValidationResult { + private static final Logger logger = LoggerFactory.getLogger(ValidationResult.class); + private Resource id; private final Optional value; private final Shape shape; @@ -73,7 +77,18 @@ public ValidationResult(Value focusNode, Value value, Shape shape, if (sourceConstraintComponent.producesValidationResultValue()) { assert value != null; - this.value = Optional.of(value); + + // value could be null if assertions are disabled + // noinspection ConstantValue + if (value == null) { + logger.error( + "Source constraint component {} was expected to produce a value, but value is null! Shape: {}", + sourceConstraintComponent, shape); + } + + // value could be null if assertions are disabled + // noinspection OptionalOfNullableMisuse + this.value = Optional.ofNullable(value); } else { assert scope != ConstraintComponent.Scope.propertyShape || value == null; this.value = Optional.empty();