Skip to content

Commit 3ab03b7

Browse files
authored
Improve ShardLockObtainFailedException message (#134198)
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 e3cef72 commit 3ab03b7

File tree

3 files changed

+23
-10
lines changed

3 files changed

+23
-10
lines changed

docs/changelog/134198.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 134198
2+
summary: Improve `ShardLockObtainFailedException` message
3+
area: Store
4+
type: enhancement
5+
issues: []

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)