You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by nellboy <gi...@git.apache.org> on 2017/02/20 17:11:39 UTC

[GitHub] flink pull request #3366: Webui/watermarks tab

GitHub user nellboy opened a pull request:

    https://github.com/apache/flink/pull/3366

    Webui/watermarks tab

    implement watermarks tab and display low watermarks on graph nodes.
    ![screen shot 2017-02-20 at 17 54 51](https://cloud.githubusercontent.com/assets/39847/23135205/fb8419f8-f797-11e6-8ee3-24bd50296a77.png)


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/nellboy/flink webui/watermarks-tab

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/flink/pull/3366.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 #3366
    
----
commit 25874c6c60889d0e9dd5c491a6612957763f338f
Author: paul <ne...@gmail.com>
Date:   2017-02-16T10:25:45Z

    [FLINK-3427] [webui] implements watermarks tab

commit 0bc569635b88cd69d72f8b792862c03570dcdced
Author: paul <ne...@gmail.com>
Date:   2017-02-16T10:57:26Z

    [FLINK-3427] [webui] display watermarks index only instead of full id

commit 90f727ca1535e2d0be5302efe5b8a834d29f901c
Author: paul <ne...@gmail.com>
Date:   2017-02-20T09:54:12Z

    [FLINK-3427] [webui] replace watermarks parsing in controller with filters

commit 9fb2d4205b1e59ffe5d538fee5092abb0354ee4c
Author: paul <ne...@gmail.com>
Date:   2017-02-20T09:58:35Z

    [FLINK-3427] [webui] set watermarks to null on node change

commit 93a7a61c8b5852dbc3c86336b247d7e21421b662
Author: paul <ne...@gmail.com>
Date:   2017-02-20T17:08:45Z

    [FLINK-3427] [webui] display low watermarks on node graph

----


---
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 #3366: [FLINK-3427] [webui] watermarks tab and low waterm...

Posted by nellboy <gi...@git.apache.org>.
Github user nellboy commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3366#discussion_r102726602
  
    --- Diff: flink-runtime-web/web-dashboard/app/scripts/modules/jobs/jobs.ctrl.coffee ---
    @@ -81,6 +78,51 @@ angular.module('flinkApp')
         JobsService.stopJob($stateParams.jobid).then (data) ->
           {}
     
    +  loadJob = ()->
    +    JobsService.loadJob($stateParams.jobid).then (data) ->
    +      $scope.job = data
    +      $scope.vertices = data.vertices
    +      $scope.plan = data.plan
    +      MetricsService.setupMetrics($stateParams.jobid, data.vertices)
    +
    +  getWatermarks = (nodes)->
    +    deferred = $q.defer()
    +    watermarks = {}
    +    jid = $scope.job.jid
    +    angular.forEach nodes, (node, index) =>
    +      metricIds = []
    +      for num in [0..node.parallelism - 1]
    +        metricIds.push(num + ".currentLowWatermark")
    +      MetricsService.getMetrics(jid, node.id, metricIds).then (data) ->
    +        values = []
    +        for key, value of data.values
    +          values.push(id: key.replace('.currentLowWatermark', ''), value: value)
    +        watermarks[node.id] = values
    +        if index >= $scope.plan.nodes.length - 1
    +          deferred.resolve(watermarks)
    +    deferred.promise
    +
    +  getLowWatermarks = (watermarks)->
    --- End diff --
    
    Done


---
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 #3366: [FLINK-3427] [webui] watermarks tab and low waterm...

Posted by uce <gi...@git.apache.org>.
Github user uce commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3366#discussion_r102173431
  
    --- Diff: flink-runtime-web/web-dashboard/app/partials/jobs/job.plan.node.watermarks.jade ---
    @@ -0,0 +1,27 @@
    +//
    +  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.
    +
    +table.table.table-hover.table-clickable.table-activable.table-inner(ng-if="(watermarks | watermarksByNode:nodeid).length")
    +  thead
    +    tr
    +      th id
    --- End diff --
    
    Should be upper case. Or let's call it "Subtask".


---
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 #3366: [FLINK-3427] [webui] watermarks tab and low waterm...

Posted by nellboy <gi...@git.apache.org>.
Github user nellboy commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3366#discussion_r102726198
  
    --- Diff: flink-runtime-web/web-dashboard/app/scripts/common/filters.coffee ---
    @@ -87,3 +87,27 @@ angular.module('flinkApp')
     
     .filter "percentage", ->
       (number) -> (number * 100).toFixed(0) + '%'
    +
    +.filter "parseWatermark", ->
    +  (value) ->
    +    if value <= -9223372036854776000
    +      return 'No Watermark'
    +    else
    +      return value
    +
    +.filter "lowWatermark", ->
    +  (watermarks, nodeid) ->
    +    lowWatermark = "None"
    +    if watermarks != null && watermarks[nodeid] && watermarks[nodeid].length
    +      values = (watermark.value for watermark in watermarks[nodeid])
    +      lowWatermark = Math.min.apply(null, values)
    +      if lowWatermark <= -9223372036854776000
    +        lowWatermark = "No Watermark"
    +    return lowWatermark
    +
    +.filter "watermarksByNode", ->
    +  (watermarks, nodeid) ->
    +    arr = []
    +    if watermarks != null && watermarks[nodeid] && watermarks[nodeid].length
    +      arr = watermarks[nodeid]
    --- End diff --
    
    no, because initially there are no watermarks, so we must check if they exist or not.  Nonetheless I have refactored this function and moved it to jobs.ctrl.coffee


---
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 #3366: [FLINK-3427] [webui] watermarks tab and low waterm...

Posted by uce <gi...@git.apache.org>.
Github user uce commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3366#discussion_r102175654
  
    --- Diff: flink-runtime-web/web-dashboard/app/scripts/modules/jobs/jobs.ctrl.coffee ---
    @@ -318,3 +360,7 @@ angular.module('flinkApp')
       loadMetrics() if $scope.nodeid
     
     # --------------------------------------
    +
    +.controller 'JobPlanWatermarksController', ($scope) ->
    --- End diff --
    
    Why do we need this controller?


---
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 #3366: [FLINK-3427] [webui] watermarks tab and low waterm...

Posted by uce <gi...@git.apache.org>.
Github user uce commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3366#discussion_r102174918
  
    --- Diff: flink-runtime-web/web-dashboard/app/scripts/index.coffee ---
    @@ -30,7 +30,7 @@ angular.module('flinkApp', ['ui.router', 'angularMoment', 'dndLists'])
     
     .value 'flinkConfig', {
       jobServer: ''
    -#  jobServer: 'http://localhost:8081/'
    +  # jobServer: 'http://localhost:8081/'
    --- End diff --
    
    Can we keep the indentation as before? I think Angular is a little picky with the config here and if we uncomment this line with the new indentation it will not parse the jobServer value. I'm not sure about this. If you think it's not a problem, it's OK to leave it as is.


---
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 #3366: [FLINK-3427] [webui] watermarks tab and low waterm...

Posted by nellboy <gi...@git.apache.org>.
Github user nellboy commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3366#discussion_r102726454
  
    --- Diff: flink-runtime-web/web-dashboard/app/scripts/modules/jobs/jobs.ctrl.coffee ---
    @@ -81,6 +78,51 @@ angular.module('flinkApp')
         JobsService.stopJob($stateParams.jobid).then (data) ->
           {}
     
    +  loadJob = ()->
    --- End diff --
    
    Fixed.


---
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 #3366: [FLINK-3427] [webui] watermarks tab and low waterm...

Posted by uce <gi...@git.apache.org>.
Github user uce commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3366#discussion_r102205187
  
    --- Diff: flink-runtime-web/web-dashboard/app/scripts/modules/jobs/jobs.ctrl.coffee ---
    @@ -81,6 +78,51 @@ angular.module('flinkApp')
         JobsService.stopJob($stateParams.jobid).then (data) ->
           {}
     
    +  loadJob = ()->
    +    JobsService.loadJob($stateParams.jobid).then (data) ->
    +      $scope.job = data
    +      $scope.vertices = data.vertices
    +      $scope.plan = data.plan
    +      MetricsService.setupMetrics($stateParams.jobid, data.vertices)
    +
    +  getWatermarks = (nodes)->
    --- End diff --
    
    Can we add a high level comment about what happens here?


---
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 #3366: [FLINK-3427] [webui] watermarks tab and low waterm...

Posted by nellboy <gi...@git.apache.org>.
Github user nellboy commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3366#discussion_r102726782
  
    --- Diff: flink-runtime-web/web-dashboard/app/scripts/modules/jobs/jobs.ctrl.coffee ---
    @@ -318,3 +360,7 @@ angular.module('flinkApp')
       loadMetrics() if $scope.nodeid
     
     # --------------------------------------
    +
    +.controller 'JobPlanWatermarksController', ($scope) ->
    --- End diff --
    
    This controller is no longer empty, but we still needed to register it as a controller, so yes, we needed it.


---
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 #3366: [FLINK-3427] [webui] watermarks tab and low waterm...

Posted by nellboy <gi...@git.apache.org>.
Github user nellboy commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3366#discussion_r102725620
  
    --- Diff: flink-runtime-web/web-dashboard/app/partials/jobs/job.plan.node.watermarks.jade ---
    @@ -0,0 +1,27 @@
    +//
    +  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.
    +
    +table.table.table-hover.table-clickable.table-activable.table-inner(ng-if="(watermarks | watermarksByNode:nodeid).length")
    --- End diff --
    
    Fixed with the latest push


---
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 #3366: [FLINK-3427] [webui] watermarks tab and low waterm...

Posted by uce <gi...@git.apache.org>.
Github user uce commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3366#discussion_r102175486
  
    --- Diff: flink-runtime-web/web-dashboard/app/scripts/modules/jobs/jobs.ctrl.coffee ---
    @@ -81,6 +78,51 @@ angular.module('flinkApp')
         JobsService.stopJob($stateParams.jobid).then (data) ->
           {}
     
    +  loadJob = ()->
    --- End diff --
    
    Do we call this function multiple times? If not, can we keep this as before?


---
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 #3366: [FLINK-3427] [webui] watermarks tab and low watermark dis...

Posted by uce <gi...@git.apache.org>.
Github user uce commented on the issue:

    https://github.com/apache/flink/pull/3366
  
    Thanks for addressing the comments. I added small refactorings on top of your changes here: (https://github.com/uce/flink/tree/watermarks) and will merge this later today. Thanks again for this contribution it is a really nice feature! :-)


---
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 #3366: [FLINK-3427] [webui] watermarks tab and low waterm...

Posted by nellboy <gi...@git.apache.org>.
Github user nellboy commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3366#discussion_r102725706
  
    --- Diff: flink-runtime-web/web-dashboard/app/partials/jobs/job.plan.node.watermarks.jade ---
    @@ -0,0 +1,27 @@
    +//
    +  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.
    +
    +table.table.table-hover.table-clickable.table-activable.table-inner(ng-if="(watermarks | watermarksByNode:nodeid).length")
    +  thead
    +    tr
    +      th id
    --- End diff --
    
    I'm not sure to what we're referring to here. Can you clarify?


---
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 #3366: [FLINK-3427] [webui] watermarks tab and low waterm...

Posted by nellboy <gi...@git.apache.org>.
Github user nellboy commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3366#discussion_r102726645
  
    --- Diff: flink-runtime-web/web-dashboard/app/scripts/modules/jobs/jobs.ctrl.coffee ---
    @@ -81,6 +78,51 @@ angular.module('flinkApp')
         JobsService.stopJob($stateParams.jobid).then (data) ->
           {}
     
    +  loadJob = ()->
    +    JobsService.loadJob($stateParams.jobid).then (data) ->
    +      $scope.job = data
    +      $scope.vertices = data.vertices
    +      $scope.plan = data.plan
    +      MetricsService.setupMetrics($stateParams.jobid, data.vertices)
    +
    +  getWatermarks = (nodes)->
    +    deferred = $q.defer()
    +    watermarks = {}
    +    jid = $scope.job.jid
    +    angular.forEach nodes, (node, index) =>
    +      metricIds = []
    +      for num in [0..node.parallelism - 1]
    +        metricIds.push(num + ".currentLowWatermark")
    +      MetricsService.getMetrics(jid, node.id, metricIds).then (data) ->
    +        values = []
    +        for key, value of data.values
    +          values.push(id: key.replace('.currentLowWatermark', ''), value: value)
    +        watermarks[node.id] = values
    +        if index >= $scope.plan.nodes.length - 1
    +          deferred.resolve(watermarks)
    +    deferred.promise
    +
    +  getLowWatermarks = (watermarks)->
    +    lowWatermarks = []
    +    for k,v of watermarks
    +      minValue = Math.min.apply(null,(watermark.value for watermark in v))
    +      lowWatermarks[k] = if minValue <= -9223372036854776000 || v.length == 0 then 'None' else minValue
    --- End diff --
    
    Fixed.


---
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 #3366: [FLINK-3427] [webui] watermarks tab and low waterm...

Posted by uce <gi...@git.apache.org>.
Github user uce commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3366#discussion_r102205223
  
    --- Diff: flink-runtime-web/web-dashboard/app/scripts/modules/jobs/jobs.ctrl.coffee ---
    @@ -81,6 +78,51 @@ angular.module('flinkApp')
         JobsService.stopJob($stateParams.jobid).then (data) ->
           {}
     
    +  loadJob = ()->
    +    JobsService.loadJob($stateParams.jobid).then (data) ->
    +      $scope.job = data
    +      $scope.vertices = data.vertices
    +      $scope.plan = data.plan
    +      MetricsService.setupMetrics($stateParams.jobid, data.vertices)
    +
    +  getWatermarks = (nodes)->
    +    deferred = $q.defer()
    +    watermarks = {}
    +    jid = $scope.job.jid
    +    angular.forEach nodes, (node, index) =>
    +      metricIds = []
    +      for num in [0..node.parallelism - 1]
    +        metricIds.push(num + ".currentLowWatermark")
    +      MetricsService.getMetrics(jid, node.id, metricIds).then (data) ->
    +        values = []
    +        for key, value of data.values
    +          values.push(id: key.replace('.currentLowWatermark', ''), value: value)
    +        watermarks[node.id] = values
    +        if index >= $scope.plan.nodes.length - 1
    +          deferred.resolve(watermarks)
    +    deferred.promise
    +
    +  getLowWatermarks = (watermarks)->
    --- End diff --
    
    Can we add a high level comment about what happens here?


---
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 #3366: [FLINK-3427] [webui] watermarks tab and low waterm...

Posted by uce <gi...@git.apache.org>.
Github user uce commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3366#discussion_r102174360
  
    --- Diff: flink-runtime-web/web-dashboard/app/scripts/common/filters.coffee ---
    @@ -87,3 +87,27 @@ angular.module('flinkApp')
     
     .filter "percentage", ->
       (number) -> (number * 100).toFixed(0) + '%'
    +
    +.filter "parseWatermark", ->
    +  (value) ->
    +    if value <= -9223372036854776000
    +      return 'No Watermark'
    +    else
    +      return value
    +
    +.filter "lowWatermark", ->
    +  (watermarks, nodeid) ->
    +    lowWatermark = "None"
    +    if watermarks != null && watermarks[nodeid] && watermarks[nodeid].length
    +      values = (watermark.value for watermark in watermarks[nodeid])
    +      lowWatermark = Math.min.apply(null, values)
    +      if lowWatermark <= -9223372036854776000
    +        lowWatermark = "No Watermark"
    +    return lowWatermark
    +
    +.filter "watermarksByNode", ->
    +  (watermarks, nodeid) ->
    +    arr = []
    +    if watermarks != null && watermarks[nodeid] && watermarks[nodeid].length
    +      arr = watermarks[nodeid]
    --- End diff --
    
    Can we directly return `watermarks[nodeid]` here?


---
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 #3366: [FLINK-3427] [webui] watermarks tab and low waterm...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/flink/pull/3366


---
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 #3366: [FLINK-3427] [webui] watermarks tab and low waterm...

Posted by uce <gi...@git.apache.org>.
Github user uce commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3366#discussion_r102174155
  
    --- Diff: flink-runtime-web/web-dashboard/app/partials/jobs/job.plan.node.watermarks.jade ---
    @@ -0,0 +1,27 @@
    +//
    +  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.
    +
    +table.table.table-hover.table-clickable.table-activable.table-inner(ng-if="(watermarks | watermarksByNode:nodeid).length")
    --- End diff --
    
    If I click on a node that has `None` as the low watermark (I think when no metric is exposed for the node) the sub task list still expands and is clickable. You get a grew background, but don't see anything:
    
    ![screen shot 2017-02-21 at 11 12 56](https://cloud.githubusercontent.com/assets/1756620/23160749/eadeaa4c-f827-11e6-8b8c-b7f07372c546.png)



---
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 #3366: [FLINK-3427] [webui] watermarks tab and low waterm...

Posted by nellboy <gi...@git.apache.org>.
Github user nellboy commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3366#discussion_r102726419
  
    --- Diff: flink-runtime-web/web-dashboard/app/scripts/index.coffee ---
    @@ -30,7 +30,7 @@ angular.module('flinkApp', ['ui.router', 'angularMoment', 'dndLists'])
     
     .value 'flinkConfig', {
       jobServer: ''
    -#  jobServer: 'http://localhost:8081/'
    +  # jobServer: 'http://localhost:8081/'
    --- End diff --
    
    I updated it because I had the same problem with sublime :) ... It was easier for my workflow to indent it like 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 #3366: [FLINK-3427] [webui] watermarks tab and low waterm...

Posted by nellboy <gi...@git.apache.org>.
Github user nellboy commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3366#discussion_r102725785
  
    --- Diff: flink-runtime-web/web-dashboard/app/scripts/common/filters.coffee ---
    @@ -87,3 +87,27 @@ angular.module('flinkApp')
     
     .filter "percentage", ->
       (number) -> (number * 100).toFixed(0) + '%'
    +
    +.filter "parseWatermark", ->
    +  (value) ->
    +    if value <= -9223372036854776000
    +      return 'No Watermark'
    +    else
    +      return value
    +
    +.filter "lowWatermark", ->
    +  (watermarks, nodeid) ->
    +    lowWatermark = "None"
    +    if watermarks != null && watermarks[nodeid] && watermarks[nodeid].length
    +      values = (watermark.value for watermark in watermarks[nodeid])
    +      lowWatermark = Math.min.apply(null, values)
    +      if lowWatermark <= -9223372036854776000
    +        lowWatermark = "No Watermark"
    --- End diff --
    
    Fixed with latest push


---
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 #3366: [FLINK-3427] [webui] watermarks tab and low waterm...

Posted by uce <gi...@git.apache.org>.
Github user uce commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3366#discussion_r102206279
  
    --- Diff: flink-runtime-web/web-dashboard/app/scripts/common/filters.coffee ---
    @@ -87,3 +87,27 @@ angular.module('flinkApp')
     
     .filter "percentage", ->
       (number) -> (number * 100).toFixed(0) + '%'
    +
    +.filter "parseWatermark", ->
    +  (value) ->
    +    if value <= -9223372036854776000
    +      return 'No Watermark'
    +    else
    +      return value
    +
    +.filter "lowWatermark", ->
    +  (watermarks, nodeid) ->
    +    lowWatermark = "None"
    +    if watermarks != null && watermarks[nodeid] && watermarks[nodeid].length
    +      values = (watermark.value for watermark in watermarks[nodeid])
    +      lowWatermark = Math.min.apply(null, values)
    +      if lowWatermark <= -9223372036854776000
    +        lowWatermark = "No Watermark"
    --- End diff --
    
    I think this can be confusing to users, because with the logic below it sometimes says `No watermark` or `None` depending on whether the metric was available or not. I think we should stick to one of the two. The comment about the constant applies here as well.


---
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 #3366: [FLINK-3427] [webui] watermarks tab and low waterm...

Posted by uce <gi...@git.apache.org>.
Github user uce commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3366#discussion_r102205464
  
    --- Diff: flink-runtime-web/web-dashboard/app/scripts/modules/jobs/jobs.ctrl.coffee ---
    @@ -81,6 +78,51 @@ angular.module('flinkApp')
         JobsService.stopJob($stateParams.jobid).then (data) ->
           {}
     
    +  loadJob = ()->
    +    JobsService.loadJob($stateParams.jobid).then (data) ->
    +      $scope.job = data
    +      $scope.vertices = data.vertices
    +      $scope.plan = data.plan
    +      MetricsService.setupMetrics($stateParams.jobid, data.vertices)
    +
    +  getWatermarks = (nodes)->
    +    deferred = $q.defer()
    +    watermarks = {}
    +    jid = $scope.job.jid
    +    angular.forEach nodes, (node, index) =>
    +      metricIds = []
    +      for num in [0..node.parallelism - 1]
    +        metricIds.push(num + ".currentLowWatermark")
    +      MetricsService.getMetrics(jid, node.id, metricIds).then (data) ->
    +        values = []
    +        for key, value of data.values
    +          values.push(id: key.replace('.currentLowWatermark', ''), value: value)
    +        watermarks[node.id] = values
    +        if index >= $scope.plan.nodes.length - 1
    +          deferred.resolve(watermarks)
    +    deferred.promise
    +
    +  getLowWatermarks = (watermarks)->
    +    lowWatermarks = []
    +    for k,v of watermarks
    +      minValue = Math.min.apply(null,(watermark.value for watermark in v))
    +      lowWatermarks[k] = if minValue <= -9223372036854776000 || v.length == 0 then 'None' else minValue
    --- End diff --
    
    The constant `-9223372036854776000 ` is different than the returned Java `Long.MIN_VALUE`, because of differences of the internal data representation for numbers in Java and JavaScript. I think it would be good to have this constant as an extra variable with a name and comment that describes what it is (maybe `NO_WATERMARK` or so?).


---
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 #3366: [FLINK-3427] [webui] watermarks tab and low waterm...

Posted by nellboy <gi...@git.apache.org>.
Github user nellboy commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3366#discussion_r102726588
  
    --- Diff: flink-runtime-web/web-dashboard/app/scripts/modules/jobs/jobs.ctrl.coffee ---
    @@ -81,6 +78,51 @@ angular.module('flinkApp')
         JobsService.stopJob($stateParams.jobid).then (data) ->
           {}
     
    +  loadJob = ()->
    +    JobsService.loadJob($stateParams.jobid).then (data) ->
    +      $scope.job = data
    +      $scope.vertices = data.vertices
    +      $scope.plan = data.plan
    +      MetricsService.setupMetrics($stateParams.jobid, data.vertices)
    +
    +  getWatermarks = (nodes)->
    +    deferred = $q.defer()
    +    watermarks = {}
    +    jid = $scope.job.jid
    +    angular.forEach nodes, (node, index) =>
    +      metricIds = []
    +      for num in [0..node.parallelism - 1]
    +        metricIds.push(num + ".currentLowWatermark")
    +      MetricsService.getMetrics(jid, node.id, metricIds).then (data) ->
    +        values = []
    +        for key, value of data.values
    +          values.push(id: key.replace('.currentLowWatermark', ''), value: value)
    +        watermarks[node.id] = values
    +        if index >= $scope.plan.nodes.length - 1
    --- End diff --
    
    I have not fixed this, will try to see if I can find a more elegant solution.


---
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 #3366: [FLINK-3427] [webui] watermarks tab and low waterm...

Posted by uce <gi...@git.apache.org>.
Github user uce commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3366#discussion_r102208620
  
    --- Diff: flink-runtime-web/web-dashboard/app/scripts/modules/jobs/jobs.ctrl.coffee ---
    @@ -81,6 +78,51 @@ angular.module('flinkApp')
         JobsService.stopJob($stateParams.jobid).then (data) ->
           {}
     
    +  loadJob = ()->
    +    JobsService.loadJob($stateParams.jobid).then (data) ->
    +      $scope.job = data
    +      $scope.vertices = data.vertices
    +      $scope.plan = data.plan
    +      MetricsService.setupMetrics($stateParams.jobid, data.vertices)
    +
    +  getWatermarks = (nodes)->
    +    deferred = $q.defer()
    +    watermarks = {}
    +    jid = $scope.job.jid
    +    angular.forEach nodes, (node, index) =>
    +      metricIds = []
    +      for num in [0..node.parallelism - 1]
    +        metricIds.push(num + ".currentLowWatermark")
    +      MetricsService.getMetrics(jid, node.id, metricIds).then (data) ->
    +        values = []
    +        for key, value of data.values
    +          values.push(id: key.replace('.currentLowWatermark', ''), value: value)
    +        watermarks[node.id] = values
    +        if index >= $scope.plan.nodes.length - 1
    --- End diff --
    
    I'm wondering whether the idiomatic way to use promises to union them via `$q.all(promises)` and then fill watermarks when all are finished? 


---
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.
---