From b71e72f24639497668edf2403be5c8fc9bbef1e7 Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Mon, 28 Sep 2015 16:27:12 +0200 Subject: [PATCH] Adds exception objects to log messages. Also forwards exception objects through failure listeners eliminates another 17 calls to getMessage(). Relates to #10021 --- .../elasticsearch/ElasticsearchException.java | 4 ++++ .../elasticsearch/bootstrap/JNANatives.java | 2 +- .../metadata/MetaDataIndexUpgradeService.java | 2 +- .../java/org/elasticsearch/common/Base64.java | 7 ++++--- .../inject/multibindings/Multibinder.java | 2 +- .../common/inject/spi/Message.java | 4 ++++ .../discovery/zen/ZenDiscovery.java | 10 +++++----- .../zen/fd/MasterFaultDetection.java | 20 +++++++++---------- .../publish/PublishClusterStateAction.java | 19 ++++++++++++++++-- .../index/analysis/Analysis.java | 4 ++-- .../shard/TranslogRecoveryPerformer.java | 2 +- .../IndexShardSnapshotException.java | 5 +++++ .../IndexShardSnapshotFailedException.java | 4 ++++ .../BlobStoreIndexShardRepository.java | 4 ++-- .../indices/recovery/RecoveryStatus.java | 3 ++- .../indices/recovery/RecoveryTarget.java | 13 ++++++++++-- .../SharedFSRecoverySourceHandler.java | 2 +- .../transport/TransportService.java | 2 +- .../discovery/ZenFaultDetectionTests.java | 4 ++-- 19 files changed, 78 insertions(+), 35 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/ElasticsearchException.java b/core/src/main/java/org/elasticsearch/ElasticsearchException.java index 65da4ed93e1..7ae10e0a5a6 100644 --- a/core/src/main/java/org/elasticsearch/ElasticsearchException.java +++ b/core/src/main/java/org/elasticsearch/ElasticsearchException.java @@ -51,9 +51,13 @@ public class ElasticsearchException extends RuntimeException implements ToXConte private static final Map, ElasticsearchExceptionHandle> CLASS_TO_ELASTICSEARCH_EXCEPTION_HANDLE; private final Map> headers = new HashMap<>(); + /** + * Construct a ElasticsearchException with the specified cause exception. + */ public ElasticsearchException(Throwable cause) { super(cause); } + /** * Construct a ElasticsearchException with the specified detail message. * diff --git a/core/src/main/java/org/elasticsearch/bootstrap/JNANatives.java b/core/src/main/java/org/elasticsearch/bootstrap/JNANatives.java index 5db88ec254d..5356d33bb8e 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/JNANatives.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/JNANatives.java @@ -191,7 +191,7 @@ class JNANatives { if (logger.isDebugEnabled()) { logger.debug("unable to install syscall filter", t); } - logger.warn("unable to install syscall filter: " + t.getMessage()); + logger.warn("unable to install syscall filter: ", t); } } } diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexUpgradeService.java b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexUpgradeService.java index 175a6ef5c5d..9394136746a 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexUpgradeService.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexUpgradeService.java @@ -227,7 +227,7 @@ public class MetaDataIndexUpgradeService extends AbstractComponent { } } catch (Exception ex) { // Wrap the inner exception so we have the index name in the exception message - throw new IllegalStateException("unable to upgrade the mappings for the index [" + indexMetaData.getIndex() + "], reason: [" + ex.getMessage() + "]", ex); + throw new IllegalStateException("unable to upgrade the mappings for the index [" + indexMetaData.getIndex() + "]", ex); } } diff --git a/core/src/main/java/org/elasticsearch/common/Base64.java b/core/src/main/java/org/elasticsearch/common/Base64.java index 390b3708ffc..1bc6fd0ae3f 100644 --- a/core/src/main/java/org/elasticsearch/common/Base64.java +++ b/core/src/main/java/org/elasticsearch/common/Base64.java @@ -643,7 +643,8 @@ public class Base64 { try { encoded = encodeBytes(source, 0, source.length, NO_OPTIONS); } catch (java.io.IOException ex) { - assert false : ex.getMessage(); + // not sure why this was an assertion before, running with assertions disabled would mean swallowing this exception + throw new IllegalStateException(ex); } // end catch assert encoded != null; return encoded; @@ -705,7 +706,7 @@ public class Base64 { try { encoded = encodeBytes(source, off, len, NO_OPTIONS); } catch (java.io.IOException ex) { - assert false : ex.getMessage(); + throw new IllegalStateException(ex); } // end catch assert encoded != null; return encoded; @@ -766,7 +767,7 @@ public class Base64 { try { encoded = encodeBytesToBytes(source, 0, source.length, Base64.NO_OPTIONS); } catch (java.io.IOException ex) { - assert false : "IOExceptions only come from GZipping, which is turned off: " + ex.getMessage(); + throw new IllegalStateException("IOExceptions only come from GZipping, which is turned off: ", ex); } return encoded; } diff --git a/core/src/main/java/org/elasticsearch/common/inject/multibindings/Multibinder.java b/core/src/main/java/org/elasticsearch/common/inject/multibindings/Multibinder.java index 56f0ec0f055..5bc1595be5f 100644 --- a/core/src/main/java/org/elasticsearch/common/inject/multibindings/Multibinder.java +++ b/core/src/main/java/org/elasticsearch/common/inject/multibindings/Multibinder.java @@ -331,6 +331,6 @@ public abstract class Multibinder { NullPointerException npe = new NullPointerException(name); throw new ConfigurationException(singleton( - new Message(emptyList(), npe.toString(), npe))); + new Message(emptyList(), npe))); } } diff --git a/core/src/main/java/org/elasticsearch/common/inject/spi/Message.java b/core/src/main/java/org/elasticsearch/common/inject/spi/Message.java index e5488d07417..5a39b9edf13 100644 --- a/core/src/main/java/org/elasticsearch/common/inject/spi/Message.java +++ b/core/src/main/java/org/elasticsearch/common/inject/spi/Message.java @@ -58,6 +58,10 @@ public final class Message implements Serializable, Element { this(Collections.singletonList(source), message, null); } + public Message(Object source, Throwable cause) { + this(Collections.singletonList(source), null, cause); + } + public Message(String message) { this(Collections.emptyList(), message, null); } diff --git a/core/src/main/java/org/elasticsearch/discovery/zen/ZenDiscovery.java b/core/src/main/java/org/elasticsearch/discovery/zen/ZenDiscovery.java index c2b16046b0c..e5ec230fd66 100644 --- a/core/src/main/java/org/elasticsearch/discovery/zen/ZenDiscovery.java +++ b/core/src/main/java/org/elasticsearch/discovery/zen/ZenDiscovery.java @@ -525,7 +525,7 @@ public class ZenDiscovery extends AbstractLifecycleComponent implemen } }); } else if (node.equals(nodes().masterNode())) { - handleMasterGone(node, "shut_down"); + handleMasterGone(node, null, "shut_down"); } } @@ -615,7 +615,7 @@ public class ZenDiscovery extends AbstractLifecycleComponent implemen }); } - private void handleMasterGone(final DiscoveryNode masterNode, final String reason) { + private void handleMasterGone(final DiscoveryNode masterNode, final Throwable cause, final String reason) { if (lifecycleState() != Lifecycle.State.STARTED) { // not started, ignore a master failure return; @@ -625,7 +625,7 @@ public class ZenDiscovery extends AbstractLifecycleComponent implemen return; } - logger.info("master_left [{}], reason [{}]", masterNode, reason); + logger.info("master_left [{}], reason [{}]", cause, masterNode, reason); clusterService.submitStateUpdateTask("zen-disco-master_failed (" + masterNode + ")", Priority.IMMEDIATE, new ClusterStateUpdateTask() { @@ -1078,8 +1078,8 @@ public class ZenDiscovery extends AbstractLifecycleComponent implemen private class MasterNodeFailureListener implements MasterFaultDetection.Listener { @Override - public void onMasterFailure(DiscoveryNode masterNode, String reason) { - handleMasterGone(masterNode, reason); + public void onMasterFailure(DiscoveryNode masterNode, Throwable cause, String reason) { + handleMasterGone(masterNode, cause, reason); } } diff --git a/core/src/main/java/org/elasticsearch/discovery/zen/fd/MasterFaultDetection.java b/core/src/main/java/org/elasticsearch/discovery/zen/fd/MasterFaultDetection.java index 8333b967c2f..8842bafb116 100644 --- a/core/src/main/java/org/elasticsearch/discovery/zen/fd/MasterFaultDetection.java +++ b/core/src/main/java/org/elasticsearch/discovery/zen/fd/MasterFaultDetection.java @@ -49,7 +49,7 @@ public class MasterFaultDetection extends FaultDetection { public static interface Listener { /** called when pinging the master failed, like a timeout, transport disconnects etc */ - void onMasterFailure(DiscoveryNode masterNode, String reason); + void onMasterFailure(DiscoveryNode masterNode, Throwable cause, String reason); } @@ -117,7 +117,7 @@ public class MasterFaultDetection extends FaultDetection { transportService.connectToNode(masterNode); } catch (final Exception e) { // notify master failure (which stops also) and bail.. - notifyMasterFailure(masterNode, "failed to perform initial connect [" + e.getMessage() + "]"); + notifyMasterFailure(masterNode, e, "failed to perform initial connect "); return; } if (masterPinger != null) { @@ -176,22 +176,22 @@ public class MasterFaultDetection extends FaultDetection { threadPool.schedule(TimeValue.timeValueMillis(0), ThreadPool.Names.SAME, masterPinger); } catch (Exception e) { logger.trace("[master] [{}] transport disconnected (with verified connect)", masterNode); - notifyMasterFailure(masterNode, "transport disconnected (with verified connect)"); + notifyMasterFailure(masterNode, null, "transport disconnected (with verified connect)"); } } else { logger.trace("[master] [{}] transport disconnected", node); - notifyMasterFailure(node, "transport disconnected"); + notifyMasterFailure(node, null, "transport disconnected"); } } } - private void notifyMasterFailure(final DiscoveryNode masterNode, final String reason) { + private void notifyMasterFailure(final DiscoveryNode masterNode, final Throwable cause, final String reason) { if (notifiedMasterFailure.compareAndSet(false, true)) { threadPool.generic().execute(new Runnable() { @Override public void run() { for (Listener listener : listeners) { - listener.onMasterFailure(masterNode, reason); + listener.onMasterFailure(masterNode, cause, reason); } } }); @@ -255,15 +255,15 @@ public class MasterFaultDetection extends FaultDetection { return; } else if (exp.getCause() instanceof NotMasterException) { logger.debug("[master] pinging a master {} that is no longer a master", masterNode); - notifyMasterFailure(masterToPing, "no longer master"); + notifyMasterFailure(masterToPing, exp, "no longer master"); return; } else if (exp.getCause() instanceof ThisIsNotTheMasterYouAreLookingForException) { logger.debug("[master] pinging a master {} that is not the master", masterNode); - notifyMasterFailure(masterToPing, "not master"); + notifyMasterFailure(masterToPing, exp,"not master"); return; } else if (exp.getCause() instanceof NodeDoesNotExistOnMasterException) { logger.debug("[master] pinging a master {} but we do not exists on it, act as if its master failure", masterNode); - notifyMasterFailure(masterToPing, "do not exists on master, act as master failure"); + notifyMasterFailure(masterToPing, exp,"do not exists on master, act as master failure"); return; } @@ -272,7 +272,7 @@ public class MasterFaultDetection extends FaultDetection { if (retryCount >= pingRetryCount) { logger.debug("[master] failed to ping [{}], tried [{}] times, each with maximum [{}] timeout", masterNode, pingRetryCount, pingRetryTimeout); // not good, failure - notifyMasterFailure(masterToPing, "failed to ping, tried [" + pingRetryCount + "] times, each with maximum [" + pingRetryTimeout + "] timeout"); + notifyMasterFailure(masterToPing, null, "failed to ping, tried [" + pingRetryCount + "] times, each with maximum [" + pingRetryTimeout + "] timeout"); } else { // resend the request, not reschedule, rely on send timeout transportService.sendRequest(masterToPing, MASTER_PING_ACTION_NAME, request, options, this); diff --git a/core/src/main/java/org/elasticsearch/discovery/zen/publish/PublishClusterStateAction.java b/core/src/main/java/org/elasticsearch/discovery/zen/publish/PublishClusterStateAction.java index 93d457d7382..91fd622023f 100644 --- a/core/src/main/java/org/elasticsearch/discovery/zen/publish/PublishClusterStateAction.java +++ b/core/src/main/java/org/elasticsearch/discovery/zen/publish/PublishClusterStateAction.java @@ -140,9 +140,9 @@ public class PublishClusterStateAction extends AbstractComponent { throw t; } catch (Throwable t) { // try to fail committing, in cause it's still on going - if (sendingController.markAsFailed("unexpected error [" + t.getMessage() + "]")) { + if (sendingController.markAsFailed("unexpected error", t)) { // signal the change should be rejected - throw new Discovery.FailedToCommitClusterStateException("unexpected error [{}]", t, t.getMessage()); + throw new Discovery.FailedToCommitClusterStateException("unexpected error", t); } else { throw t; } @@ -583,6 +583,21 @@ public class PublishClusterStateAction extends AbstractComponent { return true; } + /** + * tries marking the publishing as failed, if a decision wasn't made yet + * + * @return true if the publishing was failed and the cluster state is *not* committed + **/ + synchronized private boolean markAsFailed(String details, Throwable reason) { + if (committedOrFailed()) { + return committed == false; + } + logger.trace("failed to commit version [{}]. {}", reason, clusterState.version(), details); + committed = false; + committedOrFailedLatch.countDown(); + return true; + } + /** * tries marking the publishing as failed, if a decision wasn't made yet * diff --git a/core/src/main/java/org/elasticsearch/index/analysis/Analysis.java b/core/src/main/java/org/elasticsearch/index/analysis/Analysis.java index 27cd9fd8c4c..a2c65c6441d 100644 --- a/core/src/main/java/org/elasticsearch/index/analysis/Analysis.java +++ b/core/src/main/java/org/elasticsearch/index/analysis/Analysis.java @@ -235,7 +235,7 @@ public class Analysis { try (BufferedReader reader = FileSystemUtils.newBufferedReader(wordListFile.toUri().toURL(), StandardCharsets.UTF_8)) { return loadWordList(reader, "#"); } catch (IOException ioe) { - String message = String.format(Locale.ROOT, "IOException while reading %s_path: %s", settingPrefix, ioe.getMessage()); + String message = String.format(Locale.ROOT, "IOException while reading %s_path: %s", settingPrefix); throw new IllegalArgumentException(message, ioe); } } @@ -282,7 +282,7 @@ public class Analysis { try { return FileSystemUtils.newBufferedReader(path.toUri().toURL(), StandardCharsets.UTF_8); } catch (IOException ioe) { - String message = String.format(Locale.ROOT, "IOException while reading %s_path: %s", settingPrefix, ioe.getMessage()); + String message = String.format(Locale.ROOT, "IOException while reading %s_path: %s", settingPrefix); throw new IllegalArgumentException(message, ioe); } } diff --git a/core/src/main/java/org/elasticsearch/index/shard/TranslogRecoveryPerformer.java b/core/src/main/java/org/elasticsearch/index/shard/TranslogRecoveryPerformer.java index 68c552d4419..ac46f6725de 100644 --- a/core/src/main/java/org/elasticsearch/index/shard/TranslogRecoveryPerformer.java +++ b/core/src/main/java/org/elasticsearch/index/shard/TranslogRecoveryPerformer.java @@ -67,7 +67,7 @@ public class TranslogRecoveryPerformer { numOps++; } } catch (Throwable t) { - throw new BatchOperationException(shardId, "failed to apply batch translog operation [" + t.getMessage() + "]", numOps, t); + throw new BatchOperationException(shardId, "failed to apply batch translog operation", numOps, t); } return numOps; } diff --git a/core/src/main/java/org/elasticsearch/index/snapshots/IndexShardSnapshotException.java b/core/src/main/java/org/elasticsearch/index/snapshots/IndexShardSnapshotException.java index 741350966a5..7ace0303f67 100644 --- a/core/src/main/java/org/elasticsearch/index/snapshots/IndexShardSnapshotException.java +++ b/core/src/main/java/org/elasticsearch/index/snapshots/IndexShardSnapshotException.java @@ -33,6 +33,11 @@ public class IndexShardSnapshotException extends ElasticsearchException { this(shardId, msg, null); } + public IndexShardSnapshotException(ShardId shardId, Throwable cause) { + super(cause); + setShard(shardId); + } + public IndexShardSnapshotException(ShardId shardId, String msg, Throwable cause) { super(msg, cause); setShard(shardId); diff --git a/core/src/main/java/org/elasticsearch/index/snapshots/IndexShardSnapshotFailedException.java b/core/src/main/java/org/elasticsearch/index/snapshots/IndexShardSnapshotFailedException.java index bfb755c9e14..7b7fc68d4d4 100644 --- a/core/src/main/java/org/elasticsearch/index/snapshots/IndexShardSnapshotFailedException.java +++ b/core/src/main/java/org/elasticsearch/index/snapshots/IndexShardSnapshotFailedException.java @@ -32,6 +32,10 @@ public class IndexShardSnapshotFailedException extends IndexShardSnapshotExcepti super(shardId, msg); } + public IndexShardSnapshotFailedException(ShardId shardId, Throwable cause) { + super(shardId, cause); + } + public IndexShardSnapshotFailedException(ShardId shardId, String msg, Throwable cause) { super(shardId, msg, cause); } diff --git a/core/src/main/java/org/elasticsearch/index/snapshots/blobstore/BlobStoreIndexShardRepository.java b/core/src/main/java/org/elasticsearch/index/snapshots/blobstore/BlobStoreIndexShardRepository.java index d90a869f5b3..674d1085660 100644 --- a/core/src/main/java/org/elasticsearch/index/snapshots/blobstore/BlobStoreIndexShardRepository.java +++ b/core/src/main/java/org/elasticsearch/index/snapshots/blobstore/BlobStoreIndexShardRepository.java @@ -191,7 +191,7 @@ public class BlobStoreIndexShardRepository extends AbstractComponent implements if (e instanceof IndexShardSnapshotFailedException) { throw (IndexShardSnapshotFailedException) e; } else { - throw new IndexShardSnapshotFailedException(shardId, e.getMessage(), e); + throw new IndexShardSnapshotFailedException(shardId, e); } } } @@ -373,7 +373,7 @@ public class BlobStoreIndexShardRepository extends AbstractComponent implements } catch (IOException e) { // We cannot delete index file - this is fatal, we cannot continue, otherwise we might end up // with references to non-existing files - throw new IndexShardSnapshotFailedException(shardId, "error deleting index files during cleanup, reason: " + e.getMessage(), e); + throw new IndexShardSnapshotFailedException(shardId, "error deleting index files during cleanup", e); } blobsToDelete = new ArrayList<>(); diff --git a/core/src/main/java/org/elasticsearch/indices/recovery/RecoveryStatus.java b/core/src/main/java/org/elasticsearch/indices/recovery/RecoveryStatus.java index 80f458b8f5c..0064021dd33 100644 --- a/core/src/main/java/org/elasticsearch/indices/recovery/RecoveryStatus.java +++ b/core/src/main/java/org/elasticsearch/indices/recovery/RecoveryStatus.java @@ -22,6 +22,7 @@ package org.elasticsearch.indices.recovery; import org.apache.lucene.store.IOContext; import org.apache.lucene.store.IndexOutput; import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.common.logging.ESLogger; import org.elasticsearch.common.logging.Loggers; @@ -175,7 +176,7 @@ public class RecoveryStatus extends AbstractRefCounted { listener.onRecoveryFailure(state(), e, sendShardFailure); } finally { try { - cancellableThreads.cancel("failed recovery [" + e.getMessage() + "]"); + cancellableThreads.cancel("failed recovery [" + ExceptionsHelper.stackTrace(e) + "]"); } finally { // release the initial reference. recovery files will be cleaned as soon as ref count goes to zero, potentially now decRef(); diff --git a/core/src/main/java/org/elasticsearch/indices/recovery/RecoveryTarget.java b/core/src/main/java/org/elasticsearch/indices/recovery/RecoveryTarget.java index 72872a585f1..2ccfbcb5420 100644 --- a/core/src/main/java/org/elasticsearch/indices/recovery/RecoveryTarget.java +++ b/core/src/main/java/org/elasticsearch/indices/recovery/RecoveryTarget.java @@ -130,8 +130,17 @@ public class RecoveryTarget extends AbstractComponent implements IndexEventListe } + protected void retryRecovery(final RecoveryStatus recoveryStatus, final Throwable reason, TimeValue retryAfter, final StartRecoveryRequest currentRequest) { + logger.trace("will retry recovery with id [{}] in [{}]", reason, recoveryStatus.recoveryId(), retryAfter); + retryRecovery(recoveryStatus, retryAfter, currentRequest); + } + protected void retryRecovery(final RecoveryStatus recoveryStatus, final String reason, TimeValue retryAfter, final StartRecoveryRequest currentRequest) { - logger.trace("will retrying recovery with id [{}] in [{}] (reason [{}])", recoveryStatus.recoveryId(), retryAfter, reason); + logger.trace("will retry recovery with id [{}] in [{}] (reason [{}])", recoveryStatus.recoveryId(), retryAfter, reason); + retryRecovery(recoveryStatus, retryAfter, currentRequest); + } + + private void retryRecovery(final RecoveryStatus recoveryStatus, TimeValue retryAfter, final StartRecoveryRequest currentRequest) { try { recoveryStatus.resetRecovery(); } catch (Throwable e) { @@ -224,7 +233,7 @@ public class RecoveryTarget extends AbstractComponent implements IndexEventListe } if (cause instanceof DelayRecoveryException) { - retryRecovery(recoveryStatus, cause.getMessage(), recoverySettings.retryDelayStateSync(), request); + retryRecovery(recoveryStatus, cause, recoverySettings.retryDelayStateSync(), request); return; } diff --git a/core/src/main/java/org/elasticsearch/indices/recovery/SharedFSRecoverySourceHandler.java b/core/src/main/java/org/elasticsearch/indices/recovery/SharedFSRecoverySourceHandler.java index 123480e81de..e849580b2c4 100644 --- a/core/src/main/java/org/elasticsearch/indices/recovery/SharedFSRecoverySourceHandler.java +++ b/core/src/main/java/org/elasticsearch/indices/recovery/SharedFSRecoverySourceHandler.java @@ -69,7 +69,7 @@ public class SharedFSRecoverySourceHandler extends RecoverySourceHandler { // create a new IndexWriter logger.info("recovery failed for primary shadow shard, failing shard"); // pass the failure as null, as we want to ensure the store is not marked as corrupted - shard.failShard("primary relocation failed on shared filesystem caused by: [" + t.getMessage() + "]", null); + shard.failShard("primary relocation failed on shared filesystem", t); } else { logger.info("recovery failed on shared filesystem", t); } diff --git a/core/src/main/java/org/elasticsearch/transport/TransportService.java b/core/src/main/java/org/elasticsearch/transport/TransportService.java index 964cfacc8c1..14fc9029b00 100644 --- a/core/src/main/java/org/elasticsearch/transport/TransportService.java +++ b/core/src/main/java/org/elasticsearch/transport/TransportService.java @@ -484,7 +484,7 @@ public class TransportService extends AbstractLifecycleComponent