Skip to content

Commit 51f3f15

Browse files
committed
Improve ShardLockObtainFailedException message
It's unclear whether the age mentioned here is the age of the lock, or just of the detailed state of the lock. This commit clarifies the message.
1 parent 1a49be5 commit 51f3f15

File tree

2 files changed

+18
-10
lines changed

2 files changed

+18
-10
lines changed

server/src/main/java/org/elasticsearch/env/NodeEnvironment.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1009,12 +1009,16 @@ void acquire(long timeoutInMillis, final String details) throws ShardLockObtainF
10091009
setDetails(details);
10101010
} else {
10111011
final Tuple<Long, String> lockDetails = this.lockDetails; // single volatile read
1012+
final var stateAge = TimeValue.timeValueNanos(System.nanoTime() - lockDetails.v1());
10121013
final var message = format(
1013-
"obtaining shard lock for [%s] timed out after [%dms], lock already held for [%s] with age [%dms]",
1014+
"""
1015+
obtaining shard lock for [%s] timed out after [%dms]; \
1016+
this shard lock is still held by a different instance of the shard and has been in state [%s] for [%s/%dms]""",
10141017
details,
10151018
timeoutInMillis,
10161019
lockDetails.v2(),
1017-
TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - lockDetails.v1())
1020+
stateAge,
1021+
stateAge.millis()
10181022
);
10191023
maybeLogThreadDump(shardId, message);
10201024
throw new ShardLockObtainFailedException(shardId, message);

server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070
import static org.hamcrest.Matchers.arrayWithSize;
7171
import static org.hamcrest.Matchers.containsString;
7272
import static org.hamcrest.Matchers.empty;
73+
import static org.hamcrest.Matchers.matchesPattern;
7374
import static org.hamcrest.Matchers.startsWith;
7475

7576
@LuceneTestCase.SuppressFileSystems("ExtrasFS") // TODO: fix test to allow extras
@@ -134,25 +135,28 @@ public void testShardLock() throws Exception {
134135

135136
try (var mockLog = MockLog.capture(NodeEnvironment.class); var lock = env.shardLock(new ShardId(index, 0), "1")) {
136137
mockLog.addExpectation(
137-
new MockLog.SeenEventExpectation(
138-
"hot threads logging",
139-
NODE_ENVIRONMENT_LOGGER_NAME,
140-
Level.DEBUG,
141-
"hot threads while failing to obtain shard lock for [foo][0]: obtaining shard lock for [2] timed out after *"
142-
)
138+
new MockLog.SeenEventExpectation("hot threads logging", NODE_ENVIRONMENT_LOGGER_NAME, Level.DEBUG, """
139+
hot threads while failing to obtain shard lock for [foo][0]: obtaining shard lock for [2] timed out after [*ms]; \
140+
this shard lock is still held by a different instance of the shard and has been in state [1] for [*/*ms]*""")
143141
);
144142
mockLog.addExpectation(
145143
new MockLog.UnseenEventExpectation(
146144
"second attempt should be suppressed due to throttling",
147145
NODE_ENVIRONMENT_LOGGER_NAME,
148146
Level.DEBUG,
149-
"hot threads while failing to obtain shard lock for [foo][0]: obtaining shard lock for [3] timed out after *"
147+
"*obtaining shard lock for [3] timed out*"
150148
)
151149
);
152150

153151
assertEquals(new ShardId(index, 0), lock.getShardId());
154152

155-
expectThrows(ShardLockObtainFailedException.class, () -> env.shardLock(new ShardId(index, 0), "2"));
153+
assertThat(
154+
expectThrows(ShardLockObtainFailedException.class, () -> env.shardLock(new ShardId(index, 0), "2")).getMessage(),
155+
matchesPattern("""
156+
\\[foo]\\[0]: obtaining shard lock for \\[2] timed out after \\[0ms]; \
157+
this shard lock is still held by a different instance of the shard \
158+
and has been in state \\[1] for \\[.*/[0-9]+ms]""")
159+
);
156160

157161
for (Path path : env.indexPaths(index)) {
158162
Files.createDirectories(path.resolve("0"));

0 commit comments

Comments
 (0)