Skip to content

Commit 16812cb

Browse files
committed
Merge branch '6.x' into ccr-6.x
* 6.x: Fix lock accounting in releasable lock text fixes (#28136) Update getting-started.asciidoc (#28145) [Docs] Delete incorrect migration notes (#27915) [Docs] Spelling fix in painless-getting-started.asciidoc (#28187) Fixed the cat.health REST test to accept 4ms, not just 4.0ms (#28186) Do not keep 5.x commits once having 6.x commits (#28188)
2 parents 9b1a28b + 0c0dc3c commit 16812cb

File tree

9 files changed

+122
-23
lines changed

9 files changed

+122
-23
lines changed

docs/painless/painless-getting-started.asciidoc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ POST hockey/player/_update_by_query
234234
----------------------------------------------------------------
235235
// CONSOLE
236236

237-
Using the match operator (`==~`) you can update all the hockey players who's
237+
Using the match operator (`==~`) you can update all the hockey players whose
238238
names start with a consonant and end with a vowel:
239239

240240
[source,js]

docs/reference/aggregations/pipeline/bucket-selector-aggregation.asciidoc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ in the parent multi-bucket aggregation. The specified metric must be numeric and
66
If the script language is `expression` then a numeric return value is permitted. In this case 0.0 will be evaluated as `false`
77
and all other values will evaluate to true.
88

9-
NOTE: The bucket_selector aggregation, like all pipeline aggregations, executions after all other sibling aggregations. This means that
9+
NOTE: The bucket_selector aggregation, like all pipeline aggregations, executes after all other sibling aggregations. This means that
1010
using the bucket_selector aggregation to filter the returned buckets in the response does not save on execution time running the aggregations.
1111

1212
==== Syntax

docs/reference/getting-started.asciidoc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -669,8 +669,8 @@ You can download the sample dataset (accounts.json) from https://github.com/elas
669669

670670
[source,sh]
671671
--------------------------------------------------
672-
curl -H "Content-Type: application/json" -XPOST 'localhost:9200/bank/account/_bulk?pretty&refresh' --data-binary "@accounts.json"
673-
curl 'localhost:9200/_cat/indices?v'
672+
curl -H "Content-Type: application/json" -XPOST "localhost:9200/bank/account/_bulk?pretty&refresh" --data-binary "@accounts.json"
673+
curl "localhost:9200/_cat/indices?v"
674674
--------------------------------------------------
675675
// NOTCONSOLE
676676

docs/reference/migration/migrate_6_0/rest.asciidoc

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,6 @@ In previous versions of Elasticsearch, Analyze API is requiring a `tokenizer` or
3838
In Elasticsearch 6.0.0, Analyze API can analyze a text as a keyword field with custom normalizer
3939
or if `char_filter`/`filter` is set and `tokenizer`/`analyzer` is not set.
4040

41-
==== Indices exists API
42-
43-
The `ignore_unavailable` and `allow_no_indices` options are no longer accepted
44-
as they could cause undesired results when their values differed from their
45-
defaults.
46-
4741
==== `timestamp` and `ttl` in index requests
4842

4943
`timestamp` and `ttl` are not accepted anymore as parameters of index/update

rest-api-spec/src/main/resources/rest-api-spec/test/cat.health/10_basic.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545
\d+ \s+ # init
4646
\d+ \s+ # unassign
4747
\d+ \s+ # pending_tasks
48-
(-|\d+[.]\d+ms|s) \s+ # max task waiting time
48+
(-|\d+(?:[.]\d+)?m?s) \s+ # max task waiting time
4949
\d+\.\d+% # active shards percent
5050
\n
5151
)+
@@ -72,7 +72,7 @@
7272
\d+ \s+ # init
7373
\d+ \s+ # unassign
7474
\d+ \s+ # pending_tasks
75-
(-|\d+[.]\d+ms|s) \s+ # max task waiting time
75+
(-|\d+(?:[.]\d+)?m?s) \s+ # max task waiting time
7676
\d+\.\d+% # active shards percent
7777
\n
7878
)+

server/src/main/java/org/elasticsearch/common/util/concurrent/ReleasableLock.java

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,9 @@
3131
public class ReleasableLock implements Releasable {
3232
private final Lock lock;
3333

34-
/* a per thread boolean indicating the lock is held by it. only works when assertions are enabled */
35-
private final ThreadLocal<Boolean> holdingThreads;
34+
35+
// a per-thread count indicating how many times the thread has entered the lock; only works if assertions are enabled
36+
private final ThreadLocal<Integer> holdingThreads;
3637

3738
public ReleasableLock(Lock lock) {
3839
this.lock = lock;
@@ -57,20 +58,27 @@ public ReleasableLock acquire() throws EngineException {
5758
}
5859

5960
private boolean addCurrentThread() {
60-
holdingThreads.set(true);
61+
final Integer current = holdingThreads.get();
62+
holdingThreads.set(current == null ? 1 : current + 1);
6163
return true;
6264
}
6365

6466
private boolean removeCurrentThread() {
65-
holdingThreads.remove();
67+
final Integer count = holdingThreads.get();
68+
assert count != null && count > 0;
69+
if (count == 1) {
70+
holdingThreads.remove();
71+
} else {
72+
holdingThreads.set(count - 1);
73+
}
6674
return true;
6775
}
6876

6977
public Boolean isHeldByCurrentThread() {
7078
if (holdingThreads == null) {
7179
throw new UnsupportedOperationException("asserts must be enabled");
7280
}
73-
Boolean b = holdingThreads.get();
74-
return b != null && b.booleanValue();
81+
final Integer count = holdingThreads.get();
82+
return count != null && count > 0;
7583
}
7684
}

server/src/main/java/org/elasticsearch/index/engine/CombinedDeletionPolicy.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,9 +121,9 @@ private static int indexOfKeptCommits(List<? extends IndexCommit> commits, long
121121
if (expectedTranslogUUID.equals(commitUserData.get(Translog.TRANSLOG_UUID_KEY)) == false) {
122122
return i + 1;
123123
}
124-
// 5.x commits do not contain MAX_SEQ_NO.
124+
// 5.x commits do not contain MAX_SEQ_NO, we should not keep it and the older commits.
125125
if (commitUserData.containsKey(SequenceNumbers.MAX_SEQ_NO) == false) {
126-
return i;
126+
return Math.min(commits.size() - 1, i + 1);
127127
}
128128
final long maxSeqNoFromCommit = Long.parseLong(commitUserData.get(SequenceNumbers.MAX_SEQ_NO));
129129
if (maxSeqNoFromCommit <= globalCheckpoint) {
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.elasticsearch.common.util.concurrent;
21+
22+
import org.elasticsearch.common.lease.Releasable;
23+
import org.elasticsearch.test.ESTestCase;
24+
25+
import java.util.ArrayList;
26+
import java.util.List;
27+
import java.util.concurrent.BrokenBarrierException;
28+
import java.util.concurrent.CyclicBarrier;
29+
import java.util.concurrent.locks.ReentrantReadWriteLock;
30+
31+
public class ReleasableLockTests extends ESTestCase {
32+
33+
/**
34+
* Test that accounting on whether or not a thread holds a releasable lock is correct. Previously we had a bug where on a re-entrant
35+
* lock that if a thread entered the lock twice we would declare that it does not hold the lock after it exits its first entrance but
36+
* not its second entrance.
37+
*
38+
* @throws BrokenBarrierException if awaiting on the synchronization barrier breaks
39+
* @throws InterruptedException if awaiting on the synchronization barrier is interrupted
40+
*/
41+
public void testIsHeldByCurrentThread() throws BrokenBarrierException, InterruptedException {
42+
final ReentrantReadWriteLock readWriteLock = new ReentrantReadWriteLock();
43+
final ReleasableLock readLock = new ReleasableLock(readWriteLock.readLock());
44+
final ReleasableLock writeLock = new ReleasableLock(readWriteLock.writeLock());
45+
46+
final int numberOfThreads = scaledRandomIntBetween(1, 32);
47+
final int iterations = scaledRandomIntBetween(1, 32);
48+
final CyclicBarrier barrier = new CyclicBarrier(1 + numberOfThreads);
49+
final List<Thread> threads = new ArrayList<>();
50+
for (int i = 0; i < numberOfThreads; i++) {
51+
final Thread thread = new Thread(() -> {
52+
try {
53+
barrier.await();
54+
} catch (final BrokenBarrierException | InterruptedException e) {
55+
throw new RuntimeException(e);
56+
}
57+
for (int j = 0; j < iterations; j++) {
58+
if (randomBoolean()) {
59+
acquire(readLock, writeLock);
60+
} else {
61+
acquire(writeLock, readLock);
62+
}
63+
}
64+
try {
65+
barrier.await();
66+
} catch (final BrokenBarrierException | InterruptedException e) {
67+
throw new RuntimeException(e);
68+
}
69+
});
70+
threads.add(thread);
71+
thread.start();
72+
}
73+
74+
barrier.await();
75+
barrier.await();
76+
for (final Thread thread : threads) {
77+
thread.join();
78+
}
79+
}
80+
81+
private void acquire(final ReleasableLock lockToAcquire, final ReleasableLock otherLock) {
82+
try (@SuppressWarnings("unused") Releasable outer = lockToAcquire.acquire()) {
83+
assertTrue(lockToAcquire.isHeldByCurrentThread());
84+
assertFalse(otherLock.isHeldByCurrentThread());
85+
try (@SuppressWarnings("unused") Releasable inner = lockToAcquire.acquire()) {
86+
assertTrue(lockToAcquire.isHeldByCurrentThread());
87+
assertFalse(otherLock.isHeldByCurrentThread());
88+
}
89+
// previously there was a bug here and this would return false
90+
assertTrue(lockToAcquire.isHeldByCurrentThread());
91+
assertFalse(otherLock.isHeldByCurrentThread());
92+
}
93+
assertFalse(lockToAcquire.isHeldByCurrentThread());
94+
assertFalse(otherLock.isHeldByCurrentThread());
95+
}
96+
97+
}

server/src/test/java/org/elasticsearch/index/engine/CombinedDeletionPolicyTests.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,15 +135,15 @@ public void testLegacyIndex() throws Exception {
135135

136136
globalCheckpoint.set(randomLongBetween(0, maxSeqNo - 1));
137137
indexPolicy.onCommit(Arrays.asList(legacyCommit, freshCommit));
138-
verify(legacyCommit, times(0)).delete();
138+
verify(legacyCommit, times(1)).delete(); // Do not keep the legacy commit once we have a new commit.
139139
verify(freshCommit, times(0)).delete();
140-
assertThat(translogPolicy.getMinTranslogGenerationForRecovery(), equalTo(legacyTranslogGen));
140+
assertThat(translogPolicy.getMinTranslogGenerationForRecovery(), equalTo(safeTranslogGen));
141141
assertThat(translogPolicy.getTranslogGenerationOfLastCommit(), equalTo(safeTranslogGen));
142142

143143
// Make the fresh commit safe.
144144
globalCheckpoint.set(randomLongBetween(maxSeqNo, Long.MAX_VALUE));
145145
indexPolicy.onCommit(Arrays.asList(legacyCommit, freshCommit));
146-
verify(legacyCommit, times(1)).delete();
146+
verify(legacyCommit, times(2)).delete();
147147
verify(freshCommit, times(0)).delete();
148148
assertThat(translogPolicy.getMinTranslogGenerationForRecovery(), equalTo(safeTranslogGen));
149149
assertThat(translogPolicy.getTranslogGenerationOfLastCommit(), equalTo(safeTranslogGen));

0 commit comments

Comments
 (0)