This is an automated email from the ASF dual-hosted git repository. marcuse pushed a commit to branch trunk in repository https://gitbox.apache.org/repos/asf/cassandra.git
commit 98d81e409a3512fccaeab3ba89f7cf5bfa8f39ae Author: Marcus Eriksson <[email protected]> AuthorDate: Fri Feb 15 10:58:09 2019 +0100 Avoid NPE in fireErrorAndComplete and make sure we log the parentSessionId and exception Patch by marcuse; reviewed by Blake Eggleston for CASSANDRA-15025 --- CHANGES.txt | 1 + src/java/org/apache/cassandra/repair/CommonRange.java | 6 +++--- src/java/org/apache/cassandra/repair/RepairRunnable.java | 16 +++++++++++----- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 0c856c2..7b06757 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.0 + * Avoid NPE in RepairRunnable.recordFailure (CASSANDRA-15025) * SSL Cert Hot Reloading should check for sanity of the new keystore/truststore before loading it (CASSANDRA-14991) * Avoid leaking threads when failing anticompactions and rate limit anticompactions (CASSANDRA-15002) * Validate token() arguments early instead of throwing NPE at execution (CASSANDRA-14989) diff --git a/src/java/org/apache/cassandra/repair/CommonRange.java b/src/java/org/apache/cassandra/repair/CommonRange.java index 6b55dc7..dab77c5 100644 --- a/src/java/org/apache/cassandra/repair/CommonRange.java +++ b/src/java/org/apache/cassandra/repair/CommonRange.java @@ -41,10 +41,10 @@ public class CommonRange public CommonRange(Set<InetAddressAndPort> endpoints, Set<InetAddressAndPort> transEndpoints, Collection<Range<Token>> ranges) { - Preconditions.checkArgument(endpoints != null && !endpoints.isEmpty()); - Preconditions.checkArgument(transEndpoints != null); + Preconditions.checkArgument(endpoints != null && !endpoints.isEmpty(), "Endpoints can not be empty"); + Preconditions.checkArgument(transEndpoints != null, "Transient endpoints can not be null"); Preconditions.checkArgument(endpoints.containsAll(transEndpoints), "transEndpoints must be a subset of endpoints"); - Preconditions.checkArgument(ranges != null && !ranges.isEmpty()); + Preconditions.checkArgument(ranges != null && !ranges.isEmpty(), "Ranges can not be empty"); this.endpoints = ImmutableSet.copyOf(endpoints); this.transEndpoints = ImmutableSet.copyOf(transEndpoints); diff --git a/src/java/org/apache/cassandra/repair/RepairRunnable.java b/src/java/org/apache/cassandra/repair/RepairRunnable.java index fa0c2a9..7e931e8 100644 --- a/src/java/org/apache/cassandra/repair/RepairRunnable.java +++ b/src/java/org/apache/cassandra/repair/RepairRunnable.java @@ -133,10 +133,11 @@ public class RepairRunnable extends WrappedRunnable implements ProgressEventNoti protected void fireErrorAndComplete(int progressCount, int totalProgress, String message) { StorageMetrics.repairExceptions.inc(); - fireProgressEvent(new ProgressEvent(ProgressEventType.ERROR, progressCount, totalProgress, message)); + String errorMessage = String.format("Repair command #%d failed with error %s", cmd, message); + fireProgressEvent(new ProgressEvent(ProgressEventType.ERROR, progressCount, totalProgress, errorMessage)); String completionMessage = String.format("Repair command #%d finished with error", cmd); fireProgressEvent(new ProgressEvent(ProgressEventType.COMPLETE, progressCount, totalProgress, completionMessage)); - recordFailure(message, completionMessage); + recordFailure(errorMessage, completionMessage); } @@ -159,7 +160,7 @@ public class RepairRunnable extends WrappedRunnable implements ProgressEventNoti } catch (IllegalArgumentException | IOException e) { - logger.error("Repair failed:", e); + logger.error("Repair {} failed:", parentSession, e); fireErrorAndComplete(progress.get(), totalProgress, e.getMessage()); return; } @@ -216,7 +217,7 @@ public class RepairRunnable extends WrappedRunnable implements ProgressEventNoti } catch (IllegalArgumentException e) { - logger.error("Repair failed:", e); + logger.error("Repair {} failed:", parentSession, e); fireErrorAndComplete(progress.get(), totalProgress, e.getMessage()); return; } @@ -230,6 +231,7 @@ public class RepairRunnable extends WrappedRunnable implements ProgressEventNoti } catch (IllegalArgumentException e) { + logger.error("Repair {} failed:", parentSession, e); fireErrorAndComplete(progress.get(), totalProgress, e.getMessage()); return; } @@ -261,6 +263,7 @@ public class RepairRunnable extends WrappedRunnable implements ProgressEventNoti } catch (Throwable t) { + logger.error("Repair {} failed:", parentSession, t); if (!options.isPreview()) { SystemDistributedKeyspace.failParentRepair(parentSession, t); @@ -639,8 +642,11 @@ public class RepairRunnable extends WrappedRunnable implements ProgressEventNoti { // Note we rely on the first message being the reason for the failure // when inspecting this state from RepairRunner.queryForCompletedRepair + String failure = failureMessage == null ? "unknown failure" : failureMessage; + String completion = completionMessage == null ? "unknown completion" : completionMessage; + ActiveRepairService.instance.recordRepairStatus(cmd, ActiveRepairService.ParentRepairStatus.FAILED, - ImmutableList.of(failureMessage, completionMessage)); + ImmutableList.of(failure, completion)); } private static void addRangeToNeighbors(List<CommonRange> neighborRangeList, Range<Token> range, EndpointsForRange neighbors) --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
