From 15b81c91f776397b7b564cf3179be5226bd2ad55 Mon Sep 17 00:00:00 2001 From: mikemccand Date: Mon, 7 Jul 2014 11:34:46 -0400 Subject: [PATCH] Core: remove per-ID locking when ID was auto-generated When we know the ID for the document we are about to index was auto-generated, we don't need to acquire/release the per-ID lock, which might provide small speedups during highly concurrent indexing. Closes #6584 --- .../index/engine/internal/InternalEngine.java | 82 ++++++++++--------- 1 file changed, 43 insertions(+), 39 deletions(-) diff --git a/src/main/java/org/elasticsearch/index/engine/internal/InternalEngine.java b/src/main/java/org/elasticsearch/index/engine/internal/InternalEngine.java index c8f0cfd9d87..2c8ee66d30f 100644 --- a/src/main/java/org/elasticsearch/index/engine/internal/InternalEngine.java +++ b/src/main/java/org/elasticsearch/index/engine/internal/InternalEngine.java @@ -402,13 +402,13 @@ public class InternalEngine extends AbstractIndexShardComponent implements Engin } private void innerCreate(Create create, IndexWriter writer) throws IOException { - synchronized (dirtyLock(create.uid())) { - final long currentVersion; - final VersionValue versionValue; - if (optimizeAutoGenerateId && create.autoGeneratedId() && !create.canHaveDuplicates()) { - currentVersion = Versions.NOT_FOUND; - versionValue = null; - } else { + if (optimizeAutoGenerateId && create.autoGeneratedId() && !create.canHaveDuplicates()) { + // We don't need to lock because this ID cannot be concurrently updated: + innerCreateNoLock(create, writer, Versions.NOT_FOUND, null); + } else { + synchronized (dirtyLock(create.uid())) { + final long currentVersion; + final VersionValue versionValue; versionValue = versionMap.getUnderLock(create.uid().bytes()); if (versionValue == null) { currentVersion = loadCurrentVersionFromIndex(create.uid()); @@ -419,51 +419,55 @@ public class InternalEngine extends AbstractIndexShardComponent implements Engin currentVersion = versionValue.version(); } } + innerCreateNoLock(create, writer, currentVersion, versionValue); } + } + } - // same logic as index - long updatedVersion; - long expectedVersion = create.version(); - if (create.versionType().isVersionConflictForWrites(currentVersion, expectedVersion)) { - if (create.origin() == Operation.Origin.RECOVERY) { - return; - } else { - throw new VersionConflictEngineException(shardId, create.type(), create.id(), currentVersion, expectedVersion); - } + private void innerCreateNoLock(Create create, IndexWriter writer, long currentVersion, VersionValue versionValue) throws IOException { + + // same logic as index + long updatedVersion; + long expectedVersion = create.version(); + if (create.versionType().isVersionConflictForWrites(currentVersion, expectedVersion)) { + if (create.origin() == Operation.Origin.RECOVERY) { + return; + } else { + throw new VersionConflictEngineException(shardId, create.type(), create.id(), currentVersion, expectedVersion); } - updatedVersion = create.versionType().updateVersion(currentVersion, expectedVersion); + } + updatedVersion = create.versionType().updateVersion(currentVersion, expectedVersion); - // if the doc does not exist or it exists but is not deleted - if (versionValue != null) { - if (!versionValue.delete()) { - if (create.origin() == Operation.Origin.RECOVERY) { - return; - } else { - throw new DocumentAlreadyExistsException(shardId, create.type(), create.id()); - } - } - } else if (currentVersion != Versions.NOT_FOUND) { - // its not deleted, its already there + // if the doc does not exist or it exists but is not deleted + if (versionValue != null) { + if (!versionValue.delete()) { if (create.origin() == Operation.Origin.RECOVERY) { return; } else { throw new DocumentAlreadyExistsException(shardId, create.type(), create.id()); } } - - create.updateVersion(updatedVersion); - - if (create.docs().size() > 1) { - writer.addDocuments(create.docs(), create.analyzer()); + } else if (currentVersion != Versions.NOT_FOUND) { + // its not deleted, its already there + if (create.origin() == Operation.Origin.RECOVERY) { + return; } else { - writer.addDocument(create.docs().get(0), create.analyzer()); + throw new DocumentAlreadyExistsException(shardId, create.type(), create.id()); } - Translog.Location translogLocation = translog.add(new Translog.Create(create)); - - versionMap.putUnderLock(create.uid().bytes(), new VersionValue(updatedVersion, translogLocation)); - - indexingService.postCreateUnderLock(create); } + + create.updateVersion(updatedVersion); + + if (create.docs().size() > 1) { + writer.addDocuments(create.docs(), create.analyzer()); + } else { + writer.addDocument(create.docs().get(0), create.analyzer()); + } + Translog.Location translogLocation = translog.add(new Translog.Create(create)); + + versionMap.putUnderLock(create.uid().bytes(), new VersionValue(updatedVersion, translogLocation)); + + indexingService.postCreateUnderLock(create); } @Override