zentol commented on code in PR #21012:
URL: https://github.com/apache/flink/pull/21012#discussion_r1016396225
##########
flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/handlers/JarRunHandler.java:
##########
@@ -130,24 +130,33 @@ private SavepointRestoreSettings
getSavepointRestoreSettings(
final JarRunRequestBody requestBody = request.getRequestBody();
+ Configuration effectiveConfiguration =
requestBody.getFlinkConfiguration();
Review Comment:
```suggestion
final Configuration configuration =
requestBody.getFlinkConfiguration();
```
Just renaming this to `effectiveConfiguration` doesn't achieve anything.
It's misleading even because it's _not_ the effective configuration.
##########
flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/handlers/JarRunHandler.java:
##########
@@ -92,12 +92,12 @@ public CompletableFuture<JarRunResponseBody> handleRequest(
@Nonnull final DispatcherGateway gateway)
throws RestHandlerException {
- final Configuration effectiveConfiguration = new
Configuration(configuration);
+ final Configuration effectiveConfiguration = new
Configuration(this.configuration);
effectiveConfiguration.set(DeploymentOptions.ATTACHED, false);
effectiveConfiguration.set(DeploymentOptions.TARGET,
EmbeddedExecutor.NAME);
final JarHandlerContext context =
JarHandlerContext.fromRequest(request, jarDir, log);
- context.applyToConfiguration(effectiveConfiguration);
+ context.applyToConfiguration(effectiveConfiguration, request);
SavepointRestoreSettings.toConfiguration(
getSavepointRestoreSettings(request), effectiveConfiguration);
Review Comment:
This doesn't behave correctly.
If a savepoint setting was not set in the request then we end up setting it
to the default value, overwriting what was configured on the cluster.
That's what I meant with "Why isn't this using the
`effectiveConfiguration`?". The fallback setting you derive in
`getSavepointRestoreSettings` (which is always used as the final setting) must
in part be based also on this `effectiveConfiguration`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
