Skip to content

Commit a587de1

Browse files
Merge branch 'main' into 2025/02/06/ES-10341-balancer-summary-data-node-changes
2 parents 7f11396 + 3f021a1 commit a587de1

File tree

47 files changed

+726
-377
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

47 files changed

+726
-377
lines changed

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ public class ElasticsearchJavaBasePlugin implements Plugin<Project> {
5858

5959
@Override
6060
public void apply(Project project) {
61-
project.getRootProject().getPlugins().apply(GlobalBuildInfoPlugin.class);
6261
// make sure the global build info plugin is applied to the root project
6362
project.getRootProject().getPluginManager().apply(GlobalBuildInfoPlugin.class);
6463
buildParams = project.getRootProject().getExtensions().getByType(BuildParameterExtension.class);

docs/changelog/121119.yaml

Lines changed: 0 additions & 5 deletions
This file was deleted.

docs/changelog/121327.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 121327
2+
summary: Reduce Data Loss in System Indices Migration
3+
area: Infra/Core
4+
type: bug
5+
issues: []

docs/changelog/122047.yaml

Lines changed: 0 additions & 5 deletions
This file was deleted.

docs/changelog/122246.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 122246
2+
summary: Ensure removal of index blocks does not leave key with null value
3+
area: Data streams
4+
type: bug
5+
issues: []

libs/entitlement/asm-provider/src/main/java/org/elasticsearch/entitlement/instrumentation/impl/InstrumentationServiceImpl.java

Lines changed: 22 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,8 @@ public InstrumentationInfo lookupImplementationMethod(
118118
) throws NoSuchMethodException, ClassNotFoundException {
119119

120120
var targetMethod = targetSuperclass.getDeclaredMethod(methodName, parameterTypes);
121-
validateTargetMethod(implementationClass, targetMethod);
121+
var implementationMethod = implementationClass.getMethod(targetMethod.getName(), targetMethod.getParameterTypes());
122+
validateTargetMethod(implementationClass, targetMethod, implementationMethod);
122123

123124
var checkerAdditionalArguments = Stream.of(Class.class, targetSuperclass);
124125
var checkMethodArgumentTypes = Stream.concat(checkerAdditionalArguments, Arrays.stream(parameterTypes))
@@ -169,15 +170,15 @@ public MethodVisitor visitMethod(
169170

170171
return new InstrumentationInfo(
171172
new MethodKey(
172-
Type.getInternalName(implementationClass),
173-
targetMethod.getName(),
173+
Type.getInternalName(implementationMethod.getDeclaringClass()),
174+
implementationMethod.getName(),
174175
Arrays.stream(parameterTypes).map(c -> Type.getType(c).getInternalName()).toList()
175176
),
176177
checkMethod[0]
177178
);
178179
}
179180

180-
private static void validateTargetMethod(Class<?> implementationClass, Method targetMethod) {
181+
private static void validateTargetMethod(Class<?> implementationClass, Method targetMethod, Method implementationMethod) {
181182
if (targetMethod.getDeclaringClass().isAssignableFrom(implementationClass) == false) {
182183
throw new IllegalArgumentException(
183184
String.format(
@@ -209,37 +210,26 @@ private static void validateTargetMethod(Class<?> implementationClass, Method ta
209210
)
210211
);
211212
}
212-
try {
213-
var implementationMethod = implementationClass.getMethod(targetMethod.getName(), targetMethod.getParameterTypes());
214-
var methodModifiers = implementationMethod.getModifiers();
215-
if (Modifier.isAbstract(methodModifiers)) {
216-
throw new IllegalArgumentException(
217-
String.format(
218-
Locale.ROOT,
219-
"Not a valid instrumentation method: %s is abstract in %s",
220-
targetMethod.getName(),
221-
implementationClass.getName()
222-
)
223-
);
224-
}
225-
if (Modifier.isPublic(methodModifiers) == false) {
226-
throw new IllegalArgumentException(
227-
String.format(
228-
Locale.ROOT,
229-
"Not a valid instrumentation method: %s is not public in %s",
230-
targetMethod.getName(),
231-
implementationClass.getName()
232-
)
233-
);
234-
}
235-
} catch (NoSuchMethodException e) {
236-
assert false
237-
: String.format(
213+
var methodModifiers = implementationMethod.getModifiers();
214+
if (Modifier.isAbstract(methodModifiers)) {
215+
throw new IllegalArgumentException(
216+
String.format(
238217
Locale.ROOT,
239-
"Not a valid instrumentation method: %s cannot be found in %s",
218+
"Not a valid instrumentation method: %s is abstract in %s",
240219
targetMethod.getName(),
241220
implementationClass.getName()
242-
);
221+
)
222+
);
223+
}
224+
if (Modifier.isPublic(methodModifiers) == false) {
225+
throw new IllegalArgumentException(
226+
String.format(
227+
Locale.ROOT,
228+
"Not a valid instrumentation method: %s is not public in %s",
229+
targetMethod.getName(),
230+
implementationClass.getName()
231+
)
232+
);
243233
}
244234
}
245235

libs/entitlement/asm-provider/src/test/java/org/elasticsearch/entitlement/instrumentation/impl/InstrumentationServiceImplTests.java

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,16 @@ public void instanceMethod(int x, String y) {}
4040

4141
abstract static class TestTargetBaseClass {
4242
abstract void instanceMethod(int x, String y);
43+
44+
abstract void instanceMethod2(int x, String y);
4345
}
4446

45-
static class TestTargetImplementationClass extends TestTargetBaseClass {
47+
abstract static class TestTargetIntermediateClass extends TestTargetBaseClass {
48+
@Override
49+
public void instanceMethod2(int x, String y) {}
50+
}
51+
52+
static class TestTargetImplementationClass extends TestTargetIntermediateClass {
4653
@Override
4754
public void instanceMethod(int x, String y) {}
4855
}
@@ -364,6 +371,44 @@ public void testLookupImplementationMethodWithBaseClass() throws ClassNotFoundEx
364371
);
365372
}
366373

374+
public void testLookupImplementationMethodWithInheritance() throws ClassNotFoundException, NoSuchMethodException {
375+
var info = instrumentationService.lookupImplementationMethod(
376+
TestTargetBaseClass.class,
377+
"instanceMethod2",
378+
TestTargetImplementationClass.class,
379+
TestCheckerMixed.class,
380+
"checkInstanceMethodManual",
381+
int.class,
382+
String.class
383+
);
384+
385+
assertThat(
386+
info.targetMethod(),
387+
equalTo(
388+
new MethodKey(
389+
"org/elasticsearch/entitlement/instrumentation/impl/InstrumentationServiceImplTests$TestTargetIntermediateClass",
390+
"instanceMethod2",
391+
List.of("I", "java/lang/String")
392+
)
393+
)
394+
);
395+
assertThat(
396+
info.checkMethod(),
397+
equalTo(
398+
new CheckMethod(
399+
"org/elasticsearch/entitlement/instrumentation/impl/InstrumentationServiceImplTests$TestCheckerMixed",
400+
"checkInstanceMethodManual",
401+
List.of(
402+
"Ljava/lang/Class;",
403+
"Lorg/elasticsearch/entitlement/instrumentation/impl/InstrumentationServiceImplTests$TestTargetBaseClass;",
404+
"I",
405+
"Ljava/lang/String;"
406+
)
407+
)
408+
)
409+
);
410+
}
411+
367412
public void testParseCheckerMethodSignatureStaticMethod() {
368413
var methodKey = InstrumentationServiceImpl.parseCheckerMethodSignature(
369414
"check$org_example_TestClass$$staticMethod",

modules/reindex/src/internalClusterTest/java/org/elasticsearch/migration/AbstractFeatureMigrationIntegTest.java

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,17 @@
99

1010
package org.elasticsearch.migration;
1111

12+
import org.elasticsearch.ElasticsearchException;
1213
import org.elasticsearch.Version;
1314
import org.elasticsearch.action.ActionListener;
15+
import org.elasticsearch.action.ActionRequest;
1416
import org.elasticsearch.action.admin.cluster.migration.TransportGetFeatureUpgradeStatusAction;
1517
import org.elasticsearch.action.admin.indices.create.CreateIndexRequestBuilder;
1618
import org.elasticsearch.action.admin.indices.create.CreateIndexResponse;
1719
import org.elasticsearch.action.admin.indices.stats.IndexStats;
1820
import org.elasticsearch.action.admin.indices.stats.IndicesStatsResponse;
1921
import org.elasticsearch.action.index.IndexRequestBuilder;
22+
import org.elasticsearch.action.support.ActionFilter;
2023
import org.elasticsearch.action.support.ActiveShardCount;
2124
import org.elasticsearch.client.internal.Client;
2225
import org.elasticsearch.cluster.ClusterState;
@@ -28,6 +31,7 @@
2831
import org.elasticsearch.index.IndexVersion;
2932
import org.elasticsearch.indices.AssociatedIndexDescriptor;
3033
import org.elasticsearch.indices.SystemIndexDescriptor;
34+
import org.elasticsearch.plugins.ActionPlugin;
3135
import org.elasticsearch.plugins.Plugin;
3236
import org.elasticsearch.plugins.PluginsService;
3337
import org.elasticsearch.plugins.SystemIndexPlugin;
@@ -50,6 +54,10 @@
5054
import java.util.function.BiConsumer;
5155
import java.util.function.Function;
5256

57+
import static java.util.Collections.emptySet;
58+
import static java.util.Collections.singletonList;
59+
import static java.util.Collections.unmodifiableSet;
60+
import static org.elasticsearch.common.util.set.Sets.newHashSet;
5361
import static org.hamcrest.Matchers.containsInAnyOrder;
5462
import static org.hamcrest.Matchers.endsWith;
5563
import static org.hamcrest.Matchers.equalTo;
@@ -255,12 +263,18 @@ protected void assertIndexHasCorrectProperties(
255263
assertThat(thisIndexStats.getTotal().getDocs().getCount(), is((long) INDEX_DOC_COUNT));
256264
}
257265

258-
public static class TestPlugin extends Plugin implements SystemIndexPlugin {
266+
public static class TestPlugin extends Plugin implements SystemIndexPlugin, ActionPlugin {
259267
public final AtomicReference<Function<ClusterState, Map<String, Object>>> preMigrationHook = new AtomicReference<>();
260268
public final AtomicReference<BiConsumer<ClusterState, Map<String, Object>>> postMigrationHook = new AtomicReference<>();
269+
private final BlockingActionFilter blockingActionFilter;
261270

262271
public TestPlugin() {
272+
blockingActionFilter = new BlockingActionFilter();
273+
}
263274

275+
@Override
276+
public List<ActionFilter> getActionFilters() {
277+
return singletonList(blockingActionFilter);
264278
}
265279

266280
@Override
@@ -299,5 +313,26 @@ public void indicesMigrationComplete(
299313
postMigrationHook.get().accept(clusterService.state(), preUpgradeMetadata);
300314
listener.onResponse(true);
301315
}
316+
317+
public static class BlockingActionFilter extends org.elasticsearch.action.support.ActionFilter.Simple {
318+
private Set<String> blockedActions = emptySet();
319+
320+
@Override
321+
protected boolean apply(String action, ActionRequest request, ActionListener<?> listener) {
322+
if (blockedActions.contains(action)) {
323+
throw new ElasticsearchException("force exception on [" + action + "]");
324+
}
325+
return true;
326+
}
327+
328+
@Override
329+
public int order() {
330+
return 0;
331+
}
332+
333+
public void blockActions(String... actions) {
334+
blockedActions = unmodifiableSet(newHashSet(actions));
335+
}
336+
}
302337
}
303338
}

modules/reindex/src/internalClusterTest/java/org/elasticsearch/migration/FeatureMigrationIT.java

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,14 @@
1717
import org.elasticsearch.action.admin.cluster.migration.PostFeatureUpgradeRequest;
1818
import org.elasticsearch.action.admin.cluster.migration.PostFeatureUpgradeResponse;
1919
import org.elasticsearch.action.admin.indices.alias.Alias;
20+
import org.elasticsearch.action.admin.indices.alias.TransportIndicesAliasesAction;
2021
import org.elasticsearch.action.admin.indices.create.CreateIndexRequestBuilder;
2122
import org.elasticsearch.action.admin.indices.create.CreateIndexResponse;
2223
import org.elasticsearch.action.admin.indices.template.put.PutComponentTemplateAction;
2324
import org.elasticsearch.action.admin.indices.template.put.TransportPutComposableIndexTemplateAction;
2425
import org.elasticsearch.action.search.SearchRequestBuilder;
26+
import org.elasticsearch.action.support.ActionFilter;
27+
import org.elasticsearch.action.support.ActionFilters;
2528
import org.elasticsearch.action.support.ActiveShardCount;
2629
import org.elasticsearch.cluster.ClusterState;
2730
import org.elasticsearch.cluster.ClusterStateUpdateTask;
@@ -36,10 +39,12 @@
3639
import org.elasticsearch.common.settings.Settings;
3740
import org.elasticsearch.index.query.QueryBuilders;
3841
import org.elasticsearch.indices.SystemIndexDescriptor;
42+
import org.elasticsearch.migration.AbstractFeatureMigrationIntegTest.TestPlugin.BlockingActionFilter;
3943
import org.elasticsearch.painless.PainlessPlugin;
4044
import org.elasticsearch.plugins.Plugin;
4145
import org.elasticsearch.plugins.SystemIndexPlugin;
4246
import org.elasticsearch.reindex.ReindexPlugin;
47+
import org.elasticsearch.test.InternalTestCluster;
4348
import org.elasticsearch.upgrades.FeatureMigrationResults;
4449
import org.elasticsearch.upgrades.SingleFeatureMigrationResult;
4550

@@ -272,6 +277,60 @@ public void testMigrateIndexWithWriteBlock() throws Exception {
272277
});
273278
}
274279

280+
@AwaitsFix(bugUrl = "ES-10666") // This test uncovered an existing issue
281+
public void testIndexBlockIsRemovedWhenAliasRequestFails() throws Exception {
282+
createSystemIndexForDescriptor(INTERNAL_UNMANAGED);
283+
ensureGreen();
284+
285+
// Block the alias request to simulate a failure
286+
InternalTestCluster internalTestCluster = internalCluster();
287+
ActionFilters actionFilters = internalTestCluster.getInstance(ActionFilters.class, internalTestCluster.getMasterName());
288+
BlockingActionFilter blockingActionFilter = null;
289+
for (ActionFilter filter : actionFilters.filters()) {
290+
if (filter instanceof BlockingActionFilter) {
291+
blockingActionFilter = (BlockingActionFilter) filter;
292+
break;
293+
}
294+
}
295+
assertNotNull("BlockingActionFilter should exist", blockingActionFilter);
296+
blockingActionFilter.blockActions(TransportIndicesAliasesAction.NAME);
297+
298+
// Start the migration
299+
client().execute(PostFeatureUpgradeAction.INSTANCE, new PostFeatureUpgradeRequest(TEST_REQUEST_TIMEOUT)).get();
300+
301+
// Wait till the migration fails
302+
assertBusy(() -> {
303+
GetFeatureUpgradeStatusResponse statusResp = client().execute(
304+
GetFeatureUpgradeStatusAction.INSTANCE,
305+
new GetFeatureUpgradeStatusRequest(TEST_REQUEST_TIMEOUT)
306+
).get();
307+
logger.info(Strings.toString(statusResp));
308+
assertThat(statusResp.getUpgradeStatus(), equalTo(GetFeatureUpgradeStatusResponse.UpgradeStatus.ERROR));
309+
});
310+
311+
// Get the settings to see if the write block was removed
312+
var allsettings = client().admin().indices().prepareGetSettings(INTERNAL_UNMANAGED.getIndexPattern()).get().getIndexToSettings();
313+
var internalUnmanagedOldIndexSettings = allsettings.get(".int-unman-old");
314+
var writeBlock = internalUnmanagedOldIndexSettings.get(IndexMetadata.INDEX_BLOCKS_WRITE_SETTING.getKey());
315+
assertThat("Write block on old index should be removed on migration ERROR status", writeBlock, equalTo("false"));
316+
317+
// Unblock the alias request
318+
blockingActionFilter.blockActions();
319+
320+
// Retry the migration
321+
client().execute(PostFeatureUpgradeAction.INSTANCE, new PostFeatureUpgradeRequest(TEST_REQUEST_TIMEOUT)).get();
322+
323+
// Ensure that the migration is successful after the alias request is unblocked
324+
assertBusy(() -> {
325+
GetFeatureUpgradeStatusResponse statusResp = client().execute(
326+
GetFeatureUpgradeStatusAction.INSTANCE,
327+
new GetFeatureUpgradeStatusRequest(TEST_REQUEST_TIMEOUT)
328+
).get();
329+
logger.info(Strings.toString(statusResp));
330+
assertThat(statusResp.getUpgradeStatus(), equalTo(GetFeatureUpgradeStatusResponse.UpgradeStatus.NO_MIGRATION_NEEDED));
331+
});
332+
}
333+
275334
public void testMigrationWillRunAfterError() throws Exception {
276335
createSystemIndexForDescriptor(INTERNAL_MANAGED);
277336

0 commit comments

Comments
 (0)