From 8128f398cac25978f2c80ad8aa21697edc2cdb6d Mon Sep 17 00:00:00 2001 From: Shay Banon Date: Thu, 7 May 2015 11:43:34 +0200 Subject: [PATCH] Fix NPE in PendingDelete#toString closes #11032 --- .../elasticsearch/indices/IndicesService.java | 57 ++++++++++++------- 1 file changed, 38 insertions(+), 19 deletions(-) diff --git a/src/main/java/org/elasticsearch/indices/IndicesService.java b/src/main/java/org/elasticsearch/indices/IndicesService.java index d932bbb3803..0d474824cf4 100644 --- a/src/main/java/org/elasticsearch/indices/IndicesService.java +++ b/src/main/java/org/elasticsearch/indices/IndicesService.java @@ -621,10 +621,18 @@ public class IndicesService extends AbstractLifecycleComponent i if (settings == null) { throw new IllegalArgumentException("settings must not be null"); } - PendingDelete pendingDelete = new PendingDelete(shardId, settings, false); + PendingDelete pendingDelete = new PendingDelete(shardId, settings); addPendingDelete(shardId.index(), pendingDelete); } + /** + * Adds a pending delete for the given index. + */ + public void addPendingDelete(Index index, @IndexSettings Settings settings) { + PendingDelete pendingDelete = new PendingDelete(index, settings); + addPendingDelete(index, pendingDelete); + } + private void addPendingDelete(Index index, PendingDelete pendingDelete) { synchronized (pendingDeletes) { List list = pendingDeletes.get(index); @@ -636,36 +644,45 @@ public class IndicesService extends AbstractLifecycleComponent i } } - /** - * Adds a pending delete for the given index shard. - */ - public void addPendingDelete(Index index, @IndexSettings Settings settings) { - PendingDelete pendingDelete = new PendingDelete(null, settings, true); - addPendingDelete(index, pendingDelete); - } - private static final class PendingDelete implements Comparable { - final ShardId shardId; + final String index; + final int shardId; final Settings settings; final boolean deleteIndex; - public PendingDelete(ShardId shardId, Settings settings, boolean deleteIndex) { - this.shardId = shardId; + /** + * Creates a new pending delete of an index + */ + public PendingDelete(ShardId shardId, Settings settings) { + this.index = shardId.getIndex(); + this.shardId = shardId.getId(); this.settings = settings; - this.deleteIndex = deleteIndex; - assert deleteIndex || shardId != null; + this.deleteIndex = false; + } + + /** + * Creates a new pending delete of a shard + */ + public PendingDelete(Index index, Settings settings) { + this.index = index.getName(); + this.shardId = -1; + this.settings = settings; + this.deleteIndex = true; } @Override public String toString() { - return shardId.toString(); + StringBuilder sb = new StringBuilder(); + sb.append("[").append(index).append("]"); + if (shardId != -1) { + sb.append("[").append(shardId).append("]"); + } + return sb.toString(); } @Override public int compareTo(PendingDelete o) { - int left = deleteIndex ? -1 : shardId.id(); - int right = o.deleteIndex ? -1 : o.shardId.id(); - return Integer.compare(left, right); + return Integer.compare(shardId, o.shardId); } } @@ -704,6 +721,7 @@ public class IndicesService extends AbstractLifecycleComponent i PendingDelete delete = iterator.next(); if (delete.deleteIndex) { + assert delete.shardId == -1; logger.debug("{} deleting index store reason [{}]", index, "pending delete"); try { nodeEnv.deleteIndexDirectoryUnderLock(index, indexSettings); @@ -712,7 +730,8 @@ public class IndicesService extends AbstractLifecycleComponent i logger.debug("{} retry pending delete", ex, index); } } else { - ShardLock shardLock = locks.get(delete.shardId); + assert delete.shardId != -1; + ShardLock shardLock = locks.get(new ShardId(delete.index, delete.shardId)); if (shardLock != null) { try { deleteShardStore("pending delete", shardLock, delete.settings);