Skip to content

Commit de247f3

Browse files
committed
Merge branch '9.2' into backport/9.2/pr-137375
2 parents 840ad49 + 509d236 commit de247f3

File tree

18 files changed

+319
-43
lines changed

18 files changed

+319
-43
lines changed

build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/ElasticsearchBuildCompletePlugin.java

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import java.util.Arrays;
4545
import java.util.List;
4646
import java.util.Optional;
47+
import java.util.concurrent.TimeUnit;
4748

4849
import javax.inject.Inject;
4950

@@ -178,7 +179,11 @@ public void execute(BuildFinishedFlowAction.Parameters parameters) throws FileNo
178179
try {
179180
// we are very generious here, as the upload can take
180181
// a long time depending on its size
181-
pb.start().waitFor(30, java.util.concurrent.TimeUnit.MINUTES);
182+
long timeoutSec = calculateUploadWaitTimeoutSeconds(uploadFile);
183+
boolean completedInTime = pb.start().waitFor(timeoutSec, TimeUnit.SECONDS);
184+
if (completedInTime == false) {
185+
System.out.println("Timed out waiting for buildkite artifact upload after " + timeoutSec + " seconds");
186+
}
182187
} catch (InterruptedException e) {
183188
System.out.println("Failed to upload buildkite artifact " + e.getMessage());
184189
}
@@ -278,5 +283,14 @@ private static void createBuildArchiveTar(List<File> files, File projectDir, Fil
278283
private static String calculateArchivePath(Path path, Path projectPath) {
279284
return path.startsWith(projectPath) ? projectPath.relativize(path).toString() : path.getFileName().toString();
280285
}
286+
287+
private static long calculateUploadWaitTimeoutSeconds(File file) {
288+
long fileSizeBytes = file.length();
289+
long fileSizeMB = fileSizeBytes / (1024 * 1024);
290+
291+
// Allocate 4 seconds per MB (assumes ~250 KB/s upload speed)
292+
// with min 10 seconds and max 30 minutes
293+
return Math.max(10, Math.min(1800, fileSizeMB * 4));
294+
}
281295
}
282296
}

docs/changelog/137047.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 137047
2+
summary: Reject invalid `reverse_nested` aggs
3+
area: Aggregations
4+
type: bug
5+
issues: []

docs/changelog/137399.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 137399
2+
summary: Allow allocating clones over low watermark
3+
area: Allocation
4+
type: bug
5+
issues: []

modules/aggregations/src/yamlRestTest/resources/rest-api-spec/test/aggregations/nested.yml

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,3 +194,33 @@ setup:
194194
- match: { aggregations.courses.highpass_filter.unnest.department.buckets.0.doc_count: 1 }
195195
- match: { aggregations.courses.highpass_filter.unnest.department.buckets.1.key: math }
196196
- match: { aggregations.courses.highpass_filter.unnest.department.buckets.1.doc_count: 1 }
197+
---
198+
"Illegal reverse nested aggregation to a child nested object":
199+
- requires:
200+
capabilities:
201+
- method: POST
202+
path: /_search
203+
capabilities: [ reject_invalid_reverse_nesting ]
204+
test_runner_features: [ capabilities ]
205+
reason: "search does not yet reject invalid reverse nesting paths"
206+
- do:
207+
catch: /Reverse nested path \[courses.sessions\] is not a parent of the current nested scope \[courses\]/
208+
search:
209+
index: test
210+
body:
211+
{
212+
"aggs": {
213+
"parent_nested": {
214+
"nested": {
215+
"path": "courses"
216+
},
217+
"aggs": {
218+
"invalid_reverse_nested": {
219+
"reverse_nested": {
220+
"path": "courses.sessions"
221+
}
222+
}
223+
}
224+
}
225+
}
226+
}

server/src/internalClusterTest/java/org/elasticsearch/cluster/routing/allocation/decider/DiskThresholdDeciderIT.java

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,18 @@
1212
import org.elasticsearch.action.admin.cluster.reroute.ClusterRerouteUtils;
1313
import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse;
1414
import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse;
15+
import org.elasticsearch.action.admin.indices.create.CreateIndexResponse;
16+
import org.elasticsearch.action.admin.indices.shrink.ResizeRequest;
17+
import org.elasticsearch.action.admin.indices.shrink.ResizeType;
1518
import org.elasticsearch.action.admin.indices.stats.ShardStats;
1619
import org.elasticsearch.action.support.ActionTestUtils;
20+
import org.elasticsearch.action.support.ActiveShardCount;
21+
import org.elasticsearch.action.support.SubscribableListener;
1722
import org.elasticsearch.cluster.ClusterInfoService;
1823
import org.elasticsearch.cluster.ClusterInfoServiceUtils;
1924
import org.elasticsearch.cluster.DiskUsageIntegTestCase;
2025
import org.elasticsearch.cluster.InternalClusterInfoService;
26+
import org.elasticsearch.cluster.metadata.IndexMetadata;
2127
import org.elasticsearch.cluster.routing.IndexRoutingTable;
2228
import org.elasticsearch.cluster.routing.IndexShardRoutingTable;
2329
import org.elasticsearch.cluster.routing.ShardRouting;
@@ -53,6 +59,7 @@
5359
import static org.elasticsearch.cluster.routing.allocation.decider.EnableAllocationDecider.CLUSTER_ROUTING_REBALANCE_ENABLE_SETTING;
5460
import static org.elasticsearch.index.store.Store.INDEX_STORE_STATS_REFRESH_INTERVAL_SETTING;
5561
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
62+
import static org.hamcrest.Matchers.allOf;
5663
import static org.hamcrest.Matchers.contains;
5764
import static org.hamcrest.Matchers.empty;
5865
import static org.hamcrest.Matchers.equalTo;
@@ -103,6 +110,62 @@ public void testHighWatermarkNotExceeded() throws Exception {
103110
assertBusyWithDiskUsageRefresh(dataNode0Id, indexName, contains(in(shardSizes.getSmallestShardIds())));
104111
}
105112

113+
public void testAllocateCloneIgnoresLowWatermark() throws Exception {
114+
final var lowWatermarkBytes = randomLongBetween(WATERMARK_BYTES + 1, WATERMARK_BYTES * 5);
115+
116+
internalCluster().startMasterOnlyNode(
117+
Settings.builder()
118+
.put(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING.getKey(), lowWatermarkBytes + "b")
119+
.build()
120+
);
121+
final var dataNodeName = internalCluster().startDataOnlyNode();
122+
ensureStableCluster(2);
123+
124+
final InternalClusterInfoService clusterInfoService = getInternalClusterInfoService();
125+
internalCluster().getCurrentMasterNodeInstance(ClusterService.class).addListener(event -> {
126+
ClusterInfoServiceUtils.refresh(clusterInfoService);
127+
});
128+
129+
final var sourceIndexName = "source-" + randomIdentifier();
130+
createIndex(sourceIndexName, indexSettings(1, 0).put(INDEX_STORE_STATS_REFRESH_INTERVAL_SETTING.getKey(), "0ms").build());
131+
final var shardSizes = createReasonableSizedShards(sourceIndexName);
132+
133+
updateIndexSettings(Settings.builder().put("blocks.write", true), sourceIndexName);
134+
135+
final var totalSpace = randomLongBetween(
136+
/* do not exceed the high watermark */
137+
shardSizes.getSmallestShardSize() + WATERMARK_BYTES + 1,
138+
/* but make it so that naively duplicating the shard would exceed the low watermark, or else it's not a meaningful test */
139+
2 * shardSizes.getSmallestShardSize() + lowWatermarkBytes
140+
);
141+
142+
getTestFileStore(dataNodeName).setTotalSpace(totalSpace);
143+
refreshDiskUsage();
144+
145+
final var targetIndexName = "target-" + randomIdentifier();
146+
final var resizeRequest = new ResizeRequest(targetIndexName, sourceIndexName);
147+
resizeRequest.setResizeType(ResizeType.CLONE);
148+
resizeRequest.masterNodeTimeout(TEST_REQUEST_TIMEOUT);
149+
resizeRequest.ackTimeout(TEST_REQUEST_TIMEOUT);
150+
resizeRequest.setWaitForActiveShards(ActiveShardCount.ALL);
151+
resizeRequest.getTargetIndexRequest()
152+
.settings(
153+
Settings.builder().put(resizeRequest.getTargetIndexRequest().settings()).put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
154+
);
155+
156+
safeAwait(
157+
SubscribableListener.<CreateIndexResponse>newForked(l -> indicesAdmin().resizeIndex(resizeRequest, l))
158+
.andThenAccept(
159+
createIndexResponse -> assertThat(
160+
true,
161+
allOf(equalTo(createIndexResponse.isAcknowledged()), equalTo(createIndexResponse.isShardsAcknowledged()))
162+
)
163+
)
164+
);
165+
166+
ensureGreen(sourceIndexName, targetIndexName);
167+
}
168+
106169
public void testRestoreSnapshotAllocationDoesNotExceedWatermark() throws Exception {
107170
internalCluster().startMasterOnlyNode();
108171
internalCluster().startDataOnlyNode();

server/src/internalClusterTest/java/org/elasticsearch/search/aggregations/bucket/ReverseNestedIT.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -470,15 +470,19 @@ public void testReverseNestedAggWithoutNestedAgg() {
470470
public void testNonExistingNestedField() throws Exception {
471471
assertNoFailuresAndResponse(
472472
prepareSearch("idx2").setQuery(matchAllQuery())
473-
.addAggregation(nested("nested2", "nested1.nested2").subAggregation(reverseNested("incorrect").path("nested3"))),
473+
.addAggregation(nested("nested2", "nested1.nested2"))
474+
.addAggregation(nested("incorrect", "nested1.incorrect")),
474475
response -> {
475476

476477
SingleBucketAggregation nested = response.getAggregations().get("nested2");
477478
assertThat(nested, notNullValue());
478479
assertThat(nested.getName(), equalTo("nested2"));
480+
assertThat(nested.getDocCount(), is(27L));
479481

480-
SingleBucketAggregation reverseNested = nested.getAggregations().get("incorrect");
481-
assertThat(reverseNested.getDocCount(), is(0L));
482+
SingleBucketAggregation incorrect = response.getAggregations().get("incorrect");
483+
assertThat(incorrect, notNullValue());
484+
assertThat(incorrect.getName(), equalTo("incorrect"));
485+
assertThat(incorrect.getDocCount(), is(0L));
482486
}
483487
);
484488

server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/DiskThresholdDecider.java

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -219,10 +219,11 @@ public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, Routing
219219
}
220220

221221
// flag that determines whether the low threshold checks below can be skipped. We use this for a primary shard that is freshly
222-
// allocated and empty.
223-
boolean skipLowThresholdChecks = shardRouting.primary()
222+
// allocated and either empty or the result of cloning another shard.
223+
final var isNewCloneTarget = isNewCloneTarget(shardRouting, allocation);
224+
final var skipLowThresholdChecks = shardRouting.primary()
224225
&& shardRouting.active() == false
225-
&& shardRouting.recoverySource().getType() == RecoverySource.Type.EMPTY_STORE;
226+
&& (isNewCloneTarget || shardRouting.recoverySource().getType() == RecoverySource.Type.EMPTY_STORE);
226227

227228
if (freeBytes < diskThresholdSettings.getFreeBytesThresholdLowStage(total).getBytes()) {
228229
if (skipLowThresholdChecks == false) {
@@ -283,6 +284,18 @@ public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, Routing
283284
}
284285

285286
// Secondly, check that allocating the shard to this node doesn't put it above the high watermark
287+
288+
if (isNewCloneTarget) {
289+
// The clone will be a hard-linked copy of the original shard so will not meaningfully increase disk usage
290+
return allocation.decision(
291+
Decision.YES,
292+
NAME,
293+
"enough disk for freshly-cloned shard on node, free: [%s], used: [%s]",
294+
freeBytesValue,
295+
Strings.format1Decimals(usedDiskPercentage, "%")
296+
);
297+
}
298+
286299
final long shardSize = getExpectedShardSize(shardRouting, 0L, allocation);
287300
assert shardSize >= 0 : shardSize;
288301
long freeBytesAfterShard = freeBytes - shardSize;
@@ -326,6 +339,23 @@ public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, Routing
326339
);
327340
}
328341

342+
private static boolean isNewCloneTarget(ShardRouting shardRouting, RoutingAllocation allocation) {
343+
if (shardRouting.unassigned() == false
344+
|| shardRouting.primary() == false
345+
|| shardRouting.recoverySource() != RecoverySource.LocalShardsRecoverySource.INSTANCE) {
346+
return false;
347+
}
348+
349+
final var targetMetadata = allocation.metadata().indexMetadata(shardRouting.index());
350+
final var sourceIndex = targetMetadata.getResizeSourceIndex();
351+
if (sourceIndex == null) {
352+
return false;
353+
}
354+
355+
final var sourceMetadata = allocation.metadata().indexMetadata(sourceIndex);
356+
return sourceMetadata != null && sourceMetadata.getNumberOfShards() == targetMetadata.getNumberOfShards();
357+
}
358+
329359
@Override
330360
public Decision canForceAllocateDuringReplace(ShardRouting shardRouting, RoutingNode node, RoutingAllocation allocation) {
331361
Map<String, DiskUsage> usages = allocation.clusterInfo().getNodeMostAvailableDiskUsages();

server/src/main/java/org/elasticsearch/rest/action/search/SearchCapabilities.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ private SearchCapabilities() {}
6060
private static final String BUCKET_SCRIPT_PARENT_MULTI_BUCKET_ERROR = "bucket_script_parent_multi_bucket_error";
6161
private static final String EXCLUDE_SOURCE_VECTORS_SETTING = "exclude_source_vectors_setting";
6262
private static final String CLUSTER_STATS_EXTENDED_USAGE = "extended-search-usage-stats";
63+
private static final String REJECT_INVALID_REVERSE_NESTING = "reject_invalid_reverse_nesting";
6364

6465
public static final Set<String> CAPABILITIES;
6566
static {
@@ -90,6 +91,7 @@ private SearchCapabilities() {}
9091
capabilities.add(BUCKET_SCRIPT_PARENT_MULTI_BUCKET_ERROR);
9192
capabilities.add(EXCLUDE_SOURCE_VECTORS_SETTING);
9293
capabilities.add(CLUSTER_STATS_EXTENDED_USAGE);
94+
capabilities.add(REJECT_INVALID_REVERSE_NESTING);
9395
CAPABILITIES = Set.copyOf(capabilities);
9496
}
9597
}

server/src/main/java/org/elasticsearch/search/aggregations/bucket/nested/ReverseNestedAggregationBuilder.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,18 @@ protected AggregatorFactory doBuild(AggregationContext context, AggregatorFactor
8484
throw new IllegalArgumentException("Reverse nested aggregation [" + name + "] can only be used inside a [nested] aggregation");
8585
}
8686

87+
if (path != null) {
88+
NestedObjectMapper currentParent = context.nestedScope().getObjectMapper();
89+
if (currentParent != null) {
90+
String parentPath = currentParent.fullPath();
91+
if (parentPath.equals(path) == false && parentPath.startsWith(path + ".") == false) {
92+
throw new IllegalArgumentException(
93+
"Reverse nested path [" + path + "] is not a parent of the current nested scope [" + parentPath + "]"
94+
);
95+
}
96+
}
97+
}
98+
8799
NestedObjectMapper nestedMapper = null;
88100
if (path != null) {
89101
nestedMapper = context.nestedLookup().getNestedMappers().get(path);
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
9204000,9185005

0 commit comments

Comments
 (0)