Race condition (consistent repro): wherein chart domain is set only after data arrives #164
Assignees
Labels
Comments
wchargin
added a commit
that referenced
this issue
Jul 6, 2017
Summary:
This fixes a number of issues with the scalar charts, some of which are
interdependent, and all of which are existing issues made more visible
by the presence of a search pane.
In particular:
1. If a chart's `run` and `tag` properties are changed simultaneously,
two updates will be triggered, and the first update will see the
data in an inconsistent state (e.g., Tag B but still Run A). We fix
the resulting issues by making observer updates asynchronous, so
that they only actually take effect once all changes have flushed.
(This fixes #169.)
2. To reduce latency due to the above solution, we further debounce
these update handlers.
3. Because the handlers are asynchronous and debounced, we need to
maintain stronger invariants on the caching keys. (In particular, if
a chart changes from Tag A to Tag B and then to Tag C, we don't want
the network requests for Tag B to fill the runs-loaded cache and
then have Tag C forgo its fetches.)
4. Furthermore, when the tag of a chart is changed, we display a
loading indicator. This is to prevent problems in the scenario where
many charts change from one tag to another: in this case, all the
charts' titles will update immediately (they're just bound to text
nodes in the DOM), but charts' data will take a while to update; the
result is that there are many charts with titles that simply do not
match their data, which is at best very confusing. The loading
indicator makes it clear that the data is stale.
5. When a chart's tag is changed, we now reset the domain of the chart
once all the data has loaded. (This fixes #164.)
6. Finally, we offer a button to manually reset the domain of the
chart. (This is often useful when changing the set of selected
runs: after adding a new run, you may need to expand the domain, but
we don't do this automatically because you may often want to keep
your old domain.)
Issues (1), (4), and (5) arise frequently when changing a search query,
as this causes many charts to change their tag and runs.
Issues (1), (2), and (4) apply to other dashboards as well, but have not
been fixed. (Issue (3) would apply to other dashboards that display one
visualization per tag instead of per run-tag combination, but there are
no such dashboards.)
Test Plan:
Cherry-pick #173 before proceeding. (Without a search feature, the
effects of this change are minimal---but I want to merge this change
before I merge the search feature!)
Launch TensorBoard with a few dozen scalar tags. Suppose without loss of
generality that the name of the fifth tag displayed in the search
results pane (on query `.*`) is `foo`. Set the search query to
`(?!foo)`; this should still match all tags. Now, add a caret at the
beginning to search for `^(?!foo)`. Charts past the fifth one should
reload. Check that the reloading behavior is pleasant.
Then, zoom into some charts, manipulating their windows. Click the
"reload" button, or wait 30 seconds, and ensure that the window is
preserved. Change the search query, though, and the windows should be
reset to show all the data.
wchargin-branch: improve-scalars-ux
wchargin
added a commit
that referenced
this issue
Jul 6, 2017
Summary:
This fixes a number of issues with the scalar charts, some of which are
interdependent, and all of which are existing issues made more visible
by the presence of a search pane.
In particular:
1. If a chart's `run` and `tag` properties are changed simultaneously,
two updates will be triggered, and the first update will see the
data in an inconsistent state (e.g., Tag B but still Run A). We fix
the resulting issues by making observer updates asynchronous, so
that they only actually take effect once all changes have flushed.
(This fixes #169.)
2. To reduce latency due to the above solution, we further debounce
these update handlers.
3. Because the handlers are asynchronous and debounced, we need to
maintain stronger invariants on the caching keys. (In particular, if
a chart changes from Tag A to Tag B and then to Tag C, we don't want
the network requests for Tag B to fill the runs-loaded cache and
then have Tag C forgo its fetches.)
4. Furthermore, when the tag of a chart is changed, we display a
loading indicator. This is to prevent problems in the scenario where
many charts change from one tag to another: in this case, all the
charts' titles will update immediately (they're just bound to text
nodes in the DOM), but charts' data will take a while to update; the
result is that there are many charts with titles that simply do not
match their data, which is at best very confusing. The loading
indicator makes it clear that the data is stale.
5. When a chart's tag is changed, we now reset the domain of the chart
once all the data has loaded. (This fixes #164.)
6. Finally, we offer a button to manually reset the domain of the
chart. (This is often useful when changing the set of selected
runs: after adding a new run, you may need to expand the domain, but
we don't do this automatically because you may often want to keep
your old domain.)
Issues (1), (4), and (5) arise frequently when changing a search query,
as this causes many charts to change their tag and runs.
Issues (1), (2), and (4) apply to other dashboards as well, but have not
been fixed. (Issue (3) would apply to other dashboards that display one
visualization per tag instead of per run-tag combination, but there are
no such dashboards.)
Test Plan:
Cherry-pick #173 before proceeding. (Without a search feature, the
effects of this change are minimal---but I want to merge this change
before I merge the search feature!)
Launch TensorBoard with a few dozen scalar tags. Suppose without loss of
generality that the name of the fifth tag displayed in the search
results pane (on query `.*`) is `foo`. Set the search query to
`(?!foo)`; this should still match all tags. Now, add a caret at the
beginning to search for `^(?!foo)`. Charts past the fifth one should
reload. Check that the reloading behavior is pleasant.
Then, zoom into some charts, manipulating their windows. Click the
"reload" button, or wait 30 seconds, and ensure that the window is
preserved. Change the search query, though, and the windows should be
reset to show all the data.
wchargin-branch: improve-scalars-ux
|
Just for posterity: the semantics that we went with are, "reset the domain when all the data loads, and not again until the tag is changed." (So if a chart loads all its data for Tag A, then it resets its domain; if later that same chart component is changed to represent Tag B, then it will reset its domain again once all its data loads.) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
A Google user reported that in some cases scalar charts appear to be
entirely empty because the domain does not include any of the data.
Toggling smoothing off (and optionally back on) solves this, as does
zooming and unzooming the chart or collapsing and expanding the pane. In
effect, anything that calls
resetYDomainfixes the problem.I did a bit of digging and determined the following. The problem
manifests in
vz-line-chart.ts. It occurs when the Polymer observer forthe
ignoreOutliersproperty fires beforesetSeriesDatahas beencalled. This can happen when the data is large or sent over a remote
connection (as opposed to localhost---the original case was on borg).
You can consistently reproduce this locally as follows:
load (should be quite fast).
throttle yourself to very slow but not offline. (Try "Good 2G"; go
lower if you can't get a repro.)
Expected behavior: Charts are blank as data loads, and then you can see
all the data once it comes in, while the charts have appropriate
domains. (This is what happens when you don't throttle the network.)
Actual behavior: Charts are blank as data loads. After network requests
resolve, chart domains are still [0, 1]. The
difference_to_ambientchart has visible data because this oscillates about the origin. On the
other hand,
currenthas no visible data because it's all around y=300.To speed up the repro, you can safely unthrottle a second or two after
data starts loading.
It's not clear to me how to design an ideal solution. We don't want the
chart to always rescale when new data arrives---this would destroy a
user's carefully positioned window every 30 seconds. Probably we want to
always rescale when new data arrives and some condition holds. The two
conditions that come to mind are "not all
visibleSerieshave had dataassigned (or this is the last one)" and "the user has not manually
specified a window by interacting with the drag-zoom-layer."
Any thoughts on what we want the semantics to be?
The text was updated successfully, but these errors were encountered: