Fix for missing physical plan in UI

[nicknezis]

Fixes #3784 and Fixes #3785

[nicknezis]

Nice catch. The tests seem to work for me, but there's definitely more to clean up as you mentioned. I guess I started making the code match this line:

incubator-heron/heron/tools/ui/resources/static/js/stat-trendlines.js

Line 264 in 4d1ab84

var u = baseUrl + '/topologies/metrics/timeline?'

with the assumption we had broken something in the Tracker cleanup. But looking back at old versions of the code, this UI query may never have matched up with the tracker api. Very odd. I personally prefer the /metrics/query syntax, but might be easier to just roll back to /metricsquery style route. And instead I'll update the javascript to use the proper api call.

[surahman]

But looking back at old versions of the code, this UI query may never have matched up with the tracker api. Very odd.

My guess is technical debt.

I personally prefer the /metrics/query syntax, but might be easier to just roll back to /metricsquery style route.

I agree. Good REST API design necessitates versioning of the API as well as nesting under path routes. All metrics queries should be nested under the /metrics/ path.

Edit: I stand corrected, this is generating a query request.

incubator-heron/heron/tools/ui/resources/static/js/plan-stats.js

Line 378 in dd23309

var url = baseUrl + '/topologies/metricstimeline?';

[surahman]

Great to pay down tech debt. I think there might be a non-functional Python docstring update required:

incubator-heron/heron/tools/tracker/src/python/query.py

Line 21 in 2baa27c

### <a name="metricsquery">Metrics Query Language</a>

[nicknezis]

I'm running into some other issues with parameter mismatch betwen ui and tracker. So I'm doing a bit of testing. One example is topology vs topology_name. There is an alias here, but it didn't seem to work. I was testing with a failed Tracker API call and got different response when using either topology or topology_name. Perhaps the alias isn't working? @Code0x58 might have some insight in helping answer that question.

Another issue I'm seeing is an exception on an API call because role isn't present. But it's listed as Optional. The parameter is treated differently between the two set of routes. Example 1 and Example 2.

[surahman]

I have always had issues bringing up the UI so I can only look at the code.

One example is topology vs topology_name. There is an alias here, but it didn't seem to work.

The naming convention for topologies has changed on K8s and I am not sure if this would affect the UI. Labels have the topology's name and the StatefulSet name has either -manager or -executors appended to it. I am not sure how the tracker manages the scraping and collecting of stats.

Another issue I'm seeing is an exception on an API call because role isn't present. But it's listed as Optional. The parameter is treated differently between the two set of routes. Example 1 and Example 2.

According to the Optional docs, a value of None will be returned as required. How is a role value of None handled? Exception? What changed?

[nicknezis]

So for testing heron-tracker and heron-ui, I realized doing the local install is much easier. You can try this:

1. bazel run --config=darwin_nostyle -- scripts/packages:heron-install.sh --user
2. Add ~/.heron/bin/ to your $PATH
3. Submit analytic:

heron submit local ~/.heron/examples/heron-streamlet-examples.jar \  org.apache.heron.examples.streamlet.WindowedWordCountTopology \
WindowedWordCountTopology \
--deploy-deactivated

4. Run the tracker: heron-tracker --type file --rootpath ~/.herondata/repository/state/local/
5. Run the UI: heron-ui

[Code0x58]

I'm running into some other issues with parameter mismatch betwen ui and tracker. So I'm doing a bit of testing. One example is topology vs topology_name. There is an alias here, but it didn't seem to work. I was testing with a failed Tracker API call and got different response when using either topology or topology_name. Perhaps the alias isn't working? @Code0x58 might have some insight in helping answer that question.

It looks like alias may be a bit of a misnomer, it may be better to call it source, if this reference covers everything

[nicknezis]

I've been banging my head against this code. Things are definitely broken. I have more drastic changes, but I haven't pushed them yet because I'm still testing. The exceptions page is still broken. Somewhere we lost the combo of component and instances query parameters.

[surahman]

I am time-constrained for the next few weeks and I still cannot bring up the UI. I typically find it faster to jump in with a debugger and find the issue causing the problem and address it there. Failing that it is time to find what caused the issue.

We need to isolate the commit in which this happened. I would start by checking out the last tagged release to see if the UI is working there. If it is, run a Git bisect between that commit and master branch's HEAD to find the problematic commit. You should have a better idea of what caused the issue. There were major changes to the backend of the UI in the PR that bumped Python to 3.8/3.9, so that is a potential candidate.

[surahman]

Incredible work!

I have not had the time to deployment test this PR and it would be nice if someone else could assist with that. There are some old review comments that still need to be addressed a handful of new ones. Overall it is pretty much in a mergeable state.

[surahman]

I just removed a reference to update the global Log that I added.

[nicknezis]

I can't figure out why the timeline is not working. I've tracked my issue to this line of code. Within the compute_max function, I can print the final dict that is returned. It has values. But when I print data, it only has an empty dict.

[surahman]

I am not sure but I do not think there is a need to convert the zipped together values into a list and then a dict. But then I also do not believe this would affect the return value.

incubator-heron/heron/tools/common/src/python/clients/tracker.py

Line 626 in 06e9f75

return dict(list(zip(keys, values)))

Maybe try this:

return dict(zip(keys, values))

[nicknezis]

I am not sure but I do not think there is a need to convert the zipped together values into a list and then a dict. But then I also do not believe this would affect the return value.

incubator-heron/heron/tools/common/src/python/clients/tracker.py

Line 626 in 06e9f75

return dict(list(zip(keys, values)))

Maybe try this:

return dict(zip(keys, values))

I think zip() returns an Iterator and the list() call converts to an actual List. I was not familiar with the zip method, but it seems to be a standard pattern. When I print the value returned from that call I can see the value. It's only when that same result is returned from the compute_max that I get an empty {} in the data variable.

[surahman]

I think zip() returns an Iterator and the list() call converts to an actual List.

I believe dict might have a constructor to handle zip return values as seen here.

[Code0x58]

I can't figure out why the timeline is not working. I've tracked my issue to this line of code. Within the compute_max function, I can print the final dict that is returned. It has values. But when I print data, it only has an empty dict.

Did you do that print within the len(filtered_ts) > 0 and len(filtered_ts[0]["timeline"]) > 0: branch of compute_max(...)? I'm wondering if the {} you are seeing is from the fall-though branch.

Beware that timelines and values in compute_max(...) are generators, so will be exhausted if you do something with them before they are used in the return dict(list(zip(keys, values))), so if you did print(dict(list(zip(keys, values)))) it would consume values and lead to zip of the keys list and an empty generator which results in an empty zip (python 3.10 adds strict=True to help detect these) and would so make the return dict(...) line return an empty dict.

p.s.

I am not sure but I do not think there is a need to convert the zipped together values into a list and then a dict. But then I also do not believe this would affect the return value.

incubator-heron/heron/tools/common/src/python/clients/tracker.py

Line 626 in 06e9f75

return dict(list(zip(keys, values)))

Maybe try this:

return dict(zip(keys, values))

both are equivalent and will be handled the same way due to python's duck typing, as both a generator (returned by zip, in type annotations it is compatible with typing.Iterator but is more strictly a typing.Generator) and list will be treated as iterables. The use of list(generator) can be faster for some cases, but constructs an object which will be discarded so can be a waste of memory.

[surahman]

Thank you for the detailed explanation!

[nicknezis]

I think this PR is finally done. I'm now able to see the timeline in the UI and most of the functionality is working (as good as it probably was working in the past). There are some super deep menus I think I'd like to clean up in the future, but this is good to go. Thank you for the help @surahman @Code0x58 and @thinker0 !

Edit: I'll add. This PR fixes the following functionality which was temporarily broken with the upgrade to Python 3. Most of the issues were related to the Heron UI and Heron Tracker APIs. Some of the issues were due to the updated code enforcing type hints in and out of methods. I've manually verified the below items.

1. Front page status circles
2. Front page timelines
3. Exception browser in the UI
4. Topology Counters
5. Container File browser
6. File stats
7. Log viewer

[thinker0]

+1

My Env Test Success

[thinker0]

#3814 typo

[surahman]

⭐ ⭐ ⭐ ⭐ ⭐ 👏