Suppress stack in VersionConflictEngineException (#62433)

`VersionConflictEngineException` is thrown on the hot path for updates,
but stack traces are expensive to compute and transport and rarely
useful for this kind of exception. This commit avoids computing the
stack trace for these exceptions.
This commit is contained in:
David Turner 2020-09-17 09:25:22 +01:00
parent 9a8225bbc1
commit 62dcc5b1ae
2 changed files with 15 additions and 1 deletions

View File

@ -57,4 +57,10 @@ public class VersionConflictEngineException extends EngineException {
public VersionConflictEngineException(StreamInput in) throws IOException { public VersionConflictEngineException(StreamInput in) throws IOException {
super(in); super(in);
} }
@Override
public synchronized Throwable fillInStackTrace() {
// This is on the hot path for updates; stack traces are expensive to compute and not very useful for VCEEs, so don't fill it in.
return this;
}
} }

View File

@ -195,6 +195,7 @@ import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.emptyArray;
import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.everyItem; import static org.hamcrest.Matchers.everyItem;
import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.greaterThan;
@ -1476,9 +1477,11 @@ public class InternalEngineTests extends EngineTestCase {
.setIfSeqNo(indexResult.getSeqNo()).setIfPrimaryTerm(primaryTerm.get() + 1), .setIfSeqNo(indexResult.getSeqNo()).setIfPrimaryTerm(primaryTerm.get() + 1),
searcherFactory)); searcherFactory));
expectThrows(VersionConflictEngineException.class, () -> engine.get(new Engine.Get(true, false, doc.type(), doc.id(), create.uid()) final VersionConflictEngineException versionConflictEngineException = expectThrows(VersionConflictEngineException.class,
() -> engine.get(new Engine.Get(true, false, doc.type(), doc.id(), create.uid())
.setIfSeqNo(indexResult.getSeqNo() + 1).setIfPrimaryTerm(primaryTerm.get() + 1), .setIfSeqNo(indexResult.getSeqNo() + 1).setIfPrimaryTerm(primaryTerm.get() + 1),
searcherFactory)); searcherFactory));
assertThat(versionConflictEngineException.getStackTrace(), emptyArray());
} }
public void testVersioningNewIndex() throws IOException { public void testVersioningNewIndex() throws IOException {
@ -2028,6 +2031,7 @@ public class InternalEngineTests extends EngineTestCase {
assertThat(result.getVersion(), equalTo(lastOpVersion)); assertThat(result.getVersion(), equalTo(lastOpVersion));
assertThat(result.getResultType(), equalTo(Engine.Result.Type.FAILURE)); assertThat(result.getResultType(), equalTo(Engine.Result.Type.FAILURE));
assertThat(result.getFailure(), instanceOf(VersionConflictEngineException.class)); assertThat(result.getFailure(), instanceOf(VersionConflictEngineException.class));
assertThat(result.getFailure().getStackTrace(), emptyArray());
} else { } else {
final Engine.IndexResult result; final Engine.IndexResult result;
if (versionedOp) { if (versionedOp) {
@ -2065,6 +2069,7 @@ public class InternalEngineTests extends EngineTestCase {
assertThat(result.getVersion(), equalTo(lastOpVersion)); assertThat(result.getVersion(), equalTo(lastOpVersion));
assertThat(result.getResultType(), equalTo(Engine.Result.Type.FAILURE)); assertThat(result.getResultType(), equalTo(Engine.Result.Type.FAILURE));
assertThat(result.getFailure(), instanceOf(VersionConflictEngineException.class)); assertThat(result.getFailure(), instanceOf(VersionConflictEngineException.class));
assertThat(result.getFailure().getStackTrace(), emptyArray());
} else { } else {
final Engine.DeleteResult result; final Engine.DeleteResult result;
long correctSeqNo = docDeleted ? UNASSIGNED_SEQ_NO : lastOpSeqNo; long correctSeqNo = docDeleted ? UNASSIGNED_SEQ_NO : lastOpSeqNo;
@ -2169,6 +2174,7 @@ public class InternalEngineTests extends EngineTestCase {
assertThat(result.getVersion(), equalTo(highestOpVersion)); assertThat(result.getVersion(), equalTo(highestOpVersion));
assertThat(result.getResultType(), equalTo(Engine.Result.Type.FAILURE)); assertThat(result.getResultType(), equalTo(Engine.Result.Type.FAILURE));
assertThat(result.getFailure(), instanceOf(VersionConflictEngineException.class)); assertThat(result.getFailure(), instanceOf(VersionConflictEngineException.class));
assertThat(result.getFailure().getStackTrace(), emptyArray());
} }
} else { } else {
final Engine.Delete delete = (Engine.Delete) op; final Engine.Delete delete = (Engine.Delete) op;
@ -2187,6 +2193,7 @@ public class InternalEngineTests extends EngineTestCase {
assertThat(result.getVersion(), equalTo(highestOpVersion)); assertThat(result.getVersion(), equalTo(highestOpVersion));
assertThat(result.getResultType(), equalTo(Engine.Result.Type.FAILURE)); assertThat(result.getResultType(), equalTo(Engine.Result.Type.FAILURE));
assertThat(result.getFailure(), instanceOf(VersionConflictEngineException.class)); assertThat(result.getFailure(), instanceOf(VersionConflictEngineException.class));
assertThat(result.getFailure().getStackTrace(), emptyArray());
} }
} }
if (randomBoolean()) { if (randomBoolean()) {
@ -4358,6 +4365,7 @@ public class InternalEngineTests extends EngineTestCase {
assertThat(operation, result.getFailure(), Matchers.instanceOf(VersionConflictEngineException.class)); assertThat(operation, result.getFailure(), Matchers.instanceOf(VersionConflictEngineException.class));
VersionConflictEngineException exception = (VersionConflictEngineException) result.getFailure(); VersionConflictEngineException exception = (VersionConflictEngineException) result.getFailure();
assertThat(operation, exception.getMessage(), containsString("but no document was found")); assertThat(operation, exception.getMessage(), containsString("but no document was found"));
assertThat(exception.getStackTrace(), emptyArray());
} }
/* /*