Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix flaky integration test TestDynamicNamespaceDelete #369

Merged
merged 1 commit into from Oct 17, 2017

Conversation

richardartoul
Copy link
Contributor

@richardartoul richardartoul commented Oct 17, 2017

The test was flaky because it was relying upon the behavior the Snapshot() functionality of the tally library.

The Snapshot() function in tally will return counter values, but only those that occurred since the last flush/report.

As a result, the test would sometimes fail due to the following race:

  1. Dynamic namespace watcher receives new value from KV
  2. Namespace watcher calls Inc(1) on "invalid-update" metric
  3. Tally "Scope" object reports metrics to reporter
  4. We called Snapshot() to check if the counter has incremented yet, but it returns 0 indefinitely because the metric has already been reported and is thus unavailable via the Snapshot() call

This PR fixes the race by relying on the TestStatsReporter instead of Snapshot(). Since the TestStatsReporter receives every report, it can keep track of every metric and the race is no longer an issue.

Copy link
Collaborator

@prateek prateek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@coveralls
Copy link

coveralls commented Oct 17, 2017

Coverage Status

Coverage decreased (-0.02%) to 78.319% when pulling 88c940e on richardartoul:ra_debug_namespace_delete into a2c7092 on m3db:master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants