Skip to content

Commit d67d13c

Browse files
committed
Revert "Fix thread context propagation in TransportClusterUpdateSettingsAction"
This reverts commit 0dc6df6.
1 parent 0dc6df6 commit d67d13c

File tree

3 files changed

+17
-58
lines changed

3 files changed

+17
-58
lines changed

muted-tests.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,9 @@ tests:
6666
- class: org.elasticsearch.xpack.test.rest.XPackRestIT
6767
method: test {p0=transform/transforms_start_stop/Test start already started transform}
6868
issue: https://github.com/elastic/elasticsearch/issues/98802
69+
- class: org.elasticsearch.xpack.deprecation.DeprecationHttpIT
70+
method: testDeprecatedSettingsReturnWarnings
71+
issue: https://github.com/elastic/elasticsearch/issues/108628
6972
- class: org.elasticsearch.xpack.shutdown.NodeShutdownIT
7073
method: testAllocationPreventedForRemoval
7174
issue: https://github.com/elastic/elasticsearch/issues/116363

server/src/main/java/org/elasticsearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import org.elasticsearch.common.settings.Settings;
3131
import org.elasticsearch.common.settings.SettingsUpdater;
3232
import org.elasticsearch.common.util.concurrent.EsExecutors;
33-
import org.elasticsearch.common.util.concurrent.ThreadContext;
3433
import org.elasticsearch.core.SuppressForbidden;
3534
import org.elasticsearch.injection.guice.Inject;
3635
import org.elasticsearch.reservedstate.action.ReservedClusterSettingsAction;
@@ -41,7 +40,6 @@
4140
import java.util.HashSet;
4241
import java.util.Optional;
4342
import java.util.Set;
44-
import java.util.function.Supplier;
4543

4644
import static org.elasticsearch.common.settings.AbstractScopedSettings.ARCHIVED_SETTINGS_PREFIX;
4745

@@ -157,8 +155,7 @@ protected void masterOperation(
157155
final ClusterState state,
158156
final ActionListener<ClusterUpdateSettingsResponse> listener
159157
) {
160-
var storedContext = threadPool.getThreadContext().newRestorableContext(false, true);
161-
var updateSettingsTask = new ClusterUpdateSettingsTask(clusterSettings, Priority.IMMEDIATE, request, storedContext, listener) {
158+
submitUnbatchedTask(UPDATE_TASK_SOURCE, new ClusterUpdateSettingsTask(clusterSettings, Priority.IMMEDIATE, request, listener) {
162159
@Override
163160
protected ClusterUpdateSettingsResponse newResponse(boolean acknowledged) {
164161
return new ClusterUpdateSettingsResponse(acknowledged, updater.getTransientUpdates(), updater.getPersistentUpdate());
@@ -231,41 +228,35 @@ public void onFailure(Exception e) {
231228
logger.debug(() -> "failed to perform [" + UPDATE_TASK_SOURCE + "]", e);
232229
super.onFailure(e);
233230
}
234-
};
235-
submitUnbatchedTask(UPDATE_TASK_SOURCE, updateSettingsTask);
231+
});
236232
}
237233

238234
private static class ClusterUpdateSettingsTask extends AckedClusterStateUpdateTask {
239235
protected volatile boolean reroute = false;
240236
protected final SettingsUpdater updater;
241237
protected final ClusterUpdateSettingsRequest request;
242-
private final Supplier<ThreadContext.StoredContext> storedContext;
243238

244239
ClusterUpdateSettingsTask(
245240
final ClusterSettings clusterSettings,
246241
Priority priority,
247242
ClusterUpdateSettingsRequest request,
248-
Supplier<ThreadContext.StoredContext> storedContext,
249243
ActionListener<? extends AcknowledgedResponse> listener
250244
) {
251245
super(priority, request, listener);
252246
this.updater = new SettingsUpdater(clusterSettings);
253247
this.request = request;
254-
this.storedContext = storedContext;
255248
}
256249

257250
@Override
258251
public ClusterState execute(final ClusterState currentState) {
259-
try (var ignored = storedContext.get()) {
260-
final ClusterState clusterState = updater.updateSettings(
261-
currentState,
262-
request.transientSettings(),
263-
request.persistentSettings(),
264-
logger
265-
);
266-
reroute = clusterState != currentState;
267-
return clusterState;
268-
}
252+
final ClusterState clusterState = updater.updateSettings(
253+
currentState,
254+
request.transientSettings(),
255+
request.persistentSettings(),
256+
logger
257+
);
258+
reroute = clusterState != currentState;
259+
return clusterState;
269260
}
270261
}
271262

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

Lines changed: 4 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -449,62 +449,27 @@ private StoredContext newStoredContext(
449449
* new Thread() {
450450
* public void run() {
451451
* try (ThreadContext.StoredContext ctx = restorable.get()) {
452-
* // execute with the parents context and restore the thread's context afterwards
453-
* // response headers of this child context will be propagated back to the parent context
454-
* // if `propagateResponseHeaders` is true
452+
* // execute with the parents context and restore the threads context afterwards
455453
* }
456454
* }
457455
*
458456
* }.start();
459457
* </pre>
460458
*
461459
* @param preserveResponseHeaders if set to <code>true</code> the response headers of the restore thread will be preserved.
462-
* @param propagateResponseHeaders if set to <code>true</code> the response headers of the child context will be propagated back.
463460
* @return a restorable context supplier
464461
*/
465-
public Supplier<StoredContext> newRestorableContext(boolean preserveResponseHeaders, boolean propagateResponseHeaders) {
466-
return wrapRestorable(
467-
preserveResponseHeaders ? newStoredContextPreservingResponseHeaders() : newStoredContext(),
468-
propagateResponseHeaders
469-
);
470-
}
471-
472-
/**
473-
* Returns a supplier that gathers a {@link #newStoredContextPreservingResponseHeaders()} and restores it once the
474-
* returned supplier is invoked. The context returned from the supplier is a stored version of the
475-
* suppliers callers context that should be restored once the originally gathered context is not needed anymore.
476-
* For instance this method should be used like this:
477-
*
478-
* <pre>
479-
* Supplier&lt;ThreadContext.StoredContext&gt; restorable = context.newRestorableContext(true);
480-
* new Thread() {
481-
* public void run() {
482-
* try (ThreadContext.StoredContext ctx = restorable.get()) {
483-
* // execute with the parents context and restore the thread's context afterwards
484-
* }
485-
* }
486-
*
487-
* }.start();
488-
* </pre>
489-
*
490-
* @param preserveExistingResponseHeaders if set to <code>true</code> the response headers of the restore thread will be preserved.
491-
* @return a restorable context supplier
492-
*/
493-
public Supplier<StoredContext> newRestorableContext(boolean preserveExistingResponseHeaders) {
494-
return newRestorableContext(preserveExistingResponseHeaders, false);
462+
public Supplier<StoredContext> newRestorableContext(boolean preserveResponseHeaders) {
463+
return wrapRestorable(preserveResponseHeaders ? newStoredContextPreservingResponseHeaders() : newStoredContext());
495464
}
496465

497466
/**
498467
* Same as {@link #newRestorableContext(boolean)} but wraps an existing context to restore.
499468
* @param storedContext the context to restore
500469
*/
501470
public Supplier<StoredContext> wrapRestorable(StoredContext storedContext) {
502-
return wrapRestorable(storedContext, false);
503-
}
504-
505-
private Supplier<StoredContext> wrapRestorable(StoredContext storedContext, boolean propagateResponseHeaders) {
506471
return () -> {
507-
var context = propagateResponseHeaders ? newStoredContextPreservingResponseHeaders() : newStoredContext();
472+
StoredContext context = newStoredContext();
508473
storedContext.restore();
509474
return context;
510475
};

0 commit comments

Comments
 (0)