You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by mushketyk <gi...@git.apache.org> on 2016/08/31 21:11:44 UTC
[GitHub] flink pull request #2448: [FLINK-4521] Enhance dashboard to show execution a...
GitHub user mushketyk opened a pull request:
https://github.com/apache/flink/pull/2448
[FLINK-4521] Enhance dashboard to show execution attempts
I've added support for displaying execution attempts in the dashboard.
The JIRA item also includes that we should add accumulators for different attempts, but it seems that accumulators are going to be deprecated (judging from this JIRA item https://issues.apache.org/jira/browse/FLINK-4527) so I didn't implement that.
Here are few screenshots of the new feature:
A list of opened attempts are collapsed:
A list of opened attempts is opened:
If there is no attempts the button is not displayed:
- [x] General
- The pull request references the related JIRA issue ("[FLINK-XXX] Jira title text")
- The pull request addresses only one issue
- Each commit in the PR has a meaningful commit message (including the JIRA id)
- [x] Documentation
- Documentation has been added for new functionality
- Old documentation affected by the pull request has been updated
- JavaDoc for public methods has been added
- [x] Tests & Build
- Functionality added by the pull request is covered by tests
- `mvn clean verify` has been executed successfully locally or a Travis build has passed
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mushketyk/flink show-attempts
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/2448.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2448
----
commit 78dac4ea8e4a73c562ce1f96cc6a220548774314
Author: Ivan Mushketyk <iv...@gmail.com>
Date: 2016-08-31T21:00:05Z
[FLINK-4521] Enhance dashboard to show execution attempts
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2448: [FLINK-3030][web frontend] Enhance dashboard to show exec...
Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on the issue:
https://github.com/apache/flink/pull/2448
Okay, I'll run gulp when merging.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2448: [FLINK-3030][web frontend] Enhance dashboard to show exec...
Posted by mushketyk <gi...@git.apache.org>.
Github user mushketyk commented on the issue:
https://github.com/apache/flink/pull/2448
Thank you for the review @StephanEwen
I'll update the PR in the evening.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #2448: [FLINK-3030][web frontend] Enhance dashboard to sh...
Posted by mushketyk <gi...@git.apache.org>.
Github user mushketyk commented on a diff in the pull request:
https://github.com/apache/flink/pull/2448#discussion_r77173124
--- Diff: flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/handlers/HandlersUtils.java ---
@@ -0,0 +1,85 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.flink.runtime.webmonitor.handlers;
+
+import com.fasterxml.jackson.core.JsonGenerator;
+import org.apache.flink.api.common.accumulators.Accumulator;
+import org.apache.flink.api.common.accumulators.LongCounter;
+import org.apache.flink.runtime.accumulators.AccumulatorRegistry;
+import org.apache.flink.runtime.execution.ExecutionState;
+import org.apache.flink.runtime.executiongraph.Execution;
+import org.apache.flink.runtime.instance.InstanceConnectionInfo;
+
+import java.io.IOException;
+import java.util.Map;
+
+public final class HandlersUtils {
--- End diff --
Good point. I would prefer to have a separate class `JsonGenerators` that will have just one responsibility.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #2448: [FLINK-3030][web frontend] Enhance dashboard to sh...
Posted by mushketyk <gi...@git.apache.org>.
Github user mushketyk commented on a diff in the pull request:
https://github.com/apache/flink/pull/2448#discussion_r77173469
--- Diff: flink-runtime-web/web-dashboard/web/css/index.css ---
@@ -228,7 +228,6 @@
overflow: hidden;
padding: 2px 4px;
font-size: 14px;
- -webkit-border-radius: 2px;
--- End diff --
Good catch. I'll take a look at what `gulp` version am I using and check how I can fix it.
Could you please check what version of `gulp` are you using? I don't think it is documented anywhere.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2448: [FLINK-3030][web frontend] Enhance dashboard to show exec...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/2448
Good to go from my side.
@rmetzger and @iampeter - since you are working on the web UI currently, can you try and merge this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #2448: [FLINK-3030][web frontend] Enhance dashboard to sh...
Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on a diff in the pull request:
https://github.com/apache/flink/pull/2448#discussion_r84459904
--- Diff: flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/handlers/JsonGenerators.java ---
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.flink.runtime.webmonitor.handlers;
+
+import com.fasterxml.jackson.core.JsonGenerator;
+import org.apache.flink.api.common.accumulators.Accumulator;
+import org.apache.flink.api.common.accumulators.LongCounter;
+import org.apache.flink.runtime.accumulators.AccumulatorRegistry;
+import org.apache.flink.runtime.execution.ExecutionState;
+import org.apache.flink.runtime.executiongraph.Execution;
+import org.apache.flink.runtime.instance.InstanceConnectionInfo;
+
+import java.io.IOException;
+import java.util.Map;
+
+public final class JsonGenerators {
+ private JsonGenerators() {}
+
+ public static void serializeExecutionAttempt(Execution execAttempt, JsonGenerator gen) throws IOException {
+
+ final ExecutionState status = execAttempt.getState();
+ final long now = System.currentTimeMillis();
+
+ InstanceConnectionInfo location = execAttempt.getAssignedResourceLocation();
+ String locationString = location == null ? "(unassigned)" : location.getHostname();
+
+ long startTime = execAttempt.getStateTimestamp(ExecutionState.DEPLOYING);
+ if (startTime == 0) {
+ startTime = -1;
+ }
+ long endTime = status.isTerminal() ? execAttempt.getStateTimestamp(status) : -1;
+ long duration = startTime > 0 ? ((endTime > 0 ? endTime : now) - startTime) : -1;
+
+ Map<AccumulatorRegistry.Metric, Accumulator<?, ?>> metrics = execAttempt.getFlinkAccumulators();
+ LongCounter readBytes;
+ LongCounter writeBytes;
+ LongCounter readRecords;
+ LongCounter writeRecords;
+
+ if (metrics != null) {
+ readBytes = (LongCounter) metrics.get(AccumulatorRegistry.Metric.NUM_BYTES_IN);
+ writeBytes = (LongCounter) metrics.get(AccumulatorRegistry.Metric.NUM_BYTES_OUT);
+ readRecords = (LongCounter) metrics.get(AccumulatorRegistry.Metric.NUM_RECORDS_IN);
+ writeRecords = (LongCounter) metrics.get(AccumulatorRegistry.Metric.NUM_RECORDS_OUT);
--- End diff --
I suspect that https://github.com/apache/flink/pull/2616 will be in conflict with this PR?
We'll have to see how we resolve that conflict.
@zentol FYI.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2448: [FLINK-3030][web frontend] Enhance dashboard to show exec...
Posted by iampeter <gi...@git.apache.org>.
Github user iampeter commented on the issue:
https://github.com/apache/flink/pull/2448
@mushketyk could you try `npm view gulp-stylus` ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2448: [FLINK-3030][web frontend] Enhance dashboard to show exec...
Posted by mushketyk <gi...@git.apache.org>.
Github user mushketyk commented on the issue:
https://github.com/apache/flink/pull/2448
Hi @uce,
I've rebased changes and updated the UI according to your suggestion. Now it looks like:
<img width="1389" alt="new attempts link" src="https://cloud.githubusercontent.com/assets/592286/22408112/2165724a-e66b-11e6-8008-6042a97ea412.png">
What do you think about this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2448: [FLINK-3030][web frontend] Enhance dashboard to show exec...
Posted by uce <gi...@git.apache.org>.
Github user uce commented on the issue:
https://github.com/apache/flink/pull/2448
It worked well, but I'm a little hesitant to merge it as it is. Running with higher parallelism than 1 I think that the button is pretty invasive:
Can we make this a little less eye-catching? Maybe just a link next to the attempt number.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2448: [FLINK-3030][web frontend] Enhance dashboard to show exec...
Posted by mushketyk <gi...@git.apache.org>.
Github user mushketyk commented on the issue:
https://github.com/apache/flink/pull/2448
@rmetzger @iampeter
I've rebased the code and updated it according to your suggestions.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2448: [FLINK-3030][web frontend] Enhance dashboard to show exec...
Posted by iampeter <gi...@git.apache.org>.
Github user iampeter commented on the issue:
https://github.com/apache/flink/pull/2448
@rmetzger will you be merging? if so, `flink-runtime-web/web-dashboard/web/js/index.js` just needs a `gulp`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #2448: [FLINK-3030][web frontend] Enhance dashboard to sh...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/2448#discussion_r77162303
--- Diff: flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/handlers/HandlersUtils.java ---
@@ -0,0 +1,85 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.flink.runtime.webmonitor.handlers;
+
+import com.fasterxml.jackson.core.JsonGenerator;
+import org.apache.flink.api.common.accumulators.Accumulator;
+import org.apache.flink.api.common.accumulators.LongCounter;
+import org.apache.flink.runtime.accumulators.AccumulatorRegistry;
+import org.apache.flink.runtime.execution.ExecutionState;
+import org.apache.flink.runtime.executiongraph.Execution;
+import org.apache.flink.runtime.instance.InstanceConnectionInfo;
+
+import java.io.IOException;
+import java.util.Map;
+
+public final class HandlersUtils {
--- End diff --
How about giving this a more revealing name. Like `JsonGenerators`?
You could also simply move the `serializeExecutionAttempt(...)` method as a static method to the `SubtaskExecutionAttemptDetailsHandler`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2448: [FLINK-3030][web frontend] Enhance dashboard to show exec...
Posted by mushketyk <gi...@git.apache.org>.
Github user mushketyk commented on the issue:
https://github.com/apache/flink/pull/2448
@StephanEwen I've renamed the class as you suggested.
Regarding the CSS change, I don't know why these changes are added. I have the required (according to README.md) `gulp` version:
```
> $ gulp -version
[21:06:35] CLI version 3.9.1
[21:06:35] Local version 3.9.1
```
I wonder if a `.styl` file was changed in one of the previous commits but it was not compiled to `.css`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #2448: [FLINK-3030][web frontend] Enhance dashboard to sh...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/2448#discussion_r77162480
--- Diff: flink-runtime-web/web-dashboard/web/css/index.css ---
@@ -228,7 +228,6 @@
overflow: hidden;
padding: 2px 4px;
font-size: 14px;
- -webkit-border-radius: 2px;
--- End diff --
How did this come to be? Are you using a different version of glup or the dependencies to build this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2448: [FLINK-3030][web frontend] Enhance dashboard to show exec...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/2448
Good change, all in all, good work!
Few minor comments, otherwise +1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #2448: [FLINK-3030][web frontend] Enhance dashboard to sh...
Posted by mushketyk <gi...@git.apache.org>.
Github user mushketyk commented on a diff in the pull request:
https://github.com/apache/flink/pull/2448#discussion_r86697108
--- Diff: flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/handlers/JsonGenerators.java ---
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.flink.runtime.webmonitor.handlers;
+
+import com.fasterxml.jackson.core.JsonGenerator;
+import org.apache.flink.api.common.accumulators.Accumulator;
+import org.apache.flink.api.common.accumulators.LongCounter;
+import org.apache.flink.runtime.accumulators.AccumulatorRegistry;
+import org.apache.flink.runtime.execution.ExecutionState;
+import org.apache.flink.runtime.executiongraph.Execution;
+import org.apache.flink.runtime.instance.InstanceConnectionInfo;
+
+import java.io.IOException;
+import java.util.Map;
+
+public final class JsonGenerators {
+ private JsonGenerators() {}
+
+ public static void serializeExecutionAttempt(Execution execAttempt, JsonGenerator gen) throws IOException {
+
+ final ExecutionState status = execAttempt.getState();
+ final long now = System.currentTimeMillis();
+
+ InstanceConnectionInfo location = execAttempt.getAssignedResourceLocation();
+ String locationString = location == null ? "(unassigned)" : location.getHostname();
+
+ long startTime = execAttempt.getStateTimestamp(ExecutionState.DEPLOYING);
+ if (startTime == 0) {
+ startTime = -1;
+ }
+ long endTime = status.isTerminal() ? execAttempt.getStateTimestamp(status) : -1;
+ long duration = startTime > 0 ? ((endTime > 0 ? endTime : now) - startTime) : -1;
+
+ Map<AccumulatorRegistry.Metric, Accumulator<?, ?>> metrics = execAttempt.getFlinkAccumulators();
+ LongCounter readBytes;
+ LongCounter writeBytes;
+ LongCounter readRecords;
+ LongCounter writeRecords;
+
+ if (metrics != null) {
+ readBytes = (LongCounter) metrics.get(AccumulatorRegistry.Metric.NUM_BYTES_IN);
+ writeBytes = (LongCounter) metrics.get(AccumulatorRegistry.Metric.NUM_BYTES_OUT);
+ readRecords = (LongCounter) metrics.get(AccumulatorRegistry.Metric.NUM_RECORDS_IN);
+ writeRecords = (LongCounter) metrics.get(AccumulatorRegistry.Metric.NUM_RECORDS_OUT);
--- End diff --
Ok. I'll rebase the change and fix merge conflicts.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2448: [FLINK-3030][web frontend] Enhance dashboard to show exec...
Posted by mushketyk <gi...@git.apache.org>.
Github user mushketyk commented on the issue:
https://github.com/apache/flink/pull/2448
@StephanEwen Should I change anything else in this PR?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2448: [FLINK-3030][web frontend] Enhance dashboard to show exec...
Posted by mushketyk <gi...@git.apache.org>.
Github user mushketyk commented on the issue:
https://github.com/apache/flink/pull/2448
Is there anything else that should be updated in this PR? It has been in review for more than a month.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2448: [FLINK-3030][web frontend] Enhance dashboard to show exec...
Posted by mushketyk <gi...@git.apache.org>.
Github user mushketyk commented on the issue:
https://github.com/apache/flink/pull/2448
Hi @uce
Thank you for your review.
I'll update the UI in the next couple of days.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---