1. Instead of the main thread sleeping for 60 seconds, created a poll cy... #33
Conversation
sn-nr
commented
Apr 8, 2015
sn-nr
commented
- Instead of the main thread sleeping for 60 seconds timer to run every 60 seconds in a separate thread.
- Added a unit test that runs for 10 minutes and verifies that 10 transmissions have occurred.
… cycle timer to run every 60 seconds in a separate thread. 2. Added a unit test that runs for 10 minutes and verifies that 10 transmissions have occurred.
|
sidekick @lars2893 |
| @@ -53,5 +54,16 @@ public void TestRunnerReportContinuesWithNegativeValue() | |||
| runner.Add(new TestAgent("FunctionalTest", -10)); | |||
| runner.SetupAndRunWithLimit(1); // Should not raise an exception | |||
| } | |||
| [TestMethod] | |||
There was a problem hiding this comment.
We should probably make this an ignored test by default since a 10 minute test is pretty hefty.
|
Hey Siva, thanks for taking this on! I'm wondering if you can help me understand how this change will fix the issue we're facing. I understand the problem is that a 60 second sleep + execution of the poll cycle will occasionally result in minute timeframes where no metrics are sent; but how does switching to a timer help? Wouldn't we still have the same 60 second timer + execution issue? I saw that there was another PR from the community about this issue: #29 . It seems to be a simpler solution, have we vetted if this will address the problem? |
|
Sorry, hasty comment, the fact that the offsets are not additive is the clear reason why this works (i.e. the timer will always trigger at a 60 second offset regardless of the previous poll cycle's execution time). I apologize; clean up the style comments, test this against the .NET Wikipedia plugin (manually just copy the DLL over and run the plugin for a while), and squash those commits and let's ship this bad boy. Great work, Siva! |
|
The idea behind the PR from the community is good (measure how long it takes to poll agents and report metrics, subtract that from 60 seconds and wait for that long), and we discussed that approach yesterday as a simpler solution requiring less code change, but the implemented solution is more rigorous and .NET style. Having said that, the actual community PR will not work because it is not implemented correctly (math is incorrect). |
|
Sorry yes the millisecond value was in the wrong place and I didn't handle the case where poll interval was greater that 60 seconds. These should now be corrected, but if not then please do add comments to that pull request. Having a poll interval greater than 60 seconds, e.g. a poll interval of 2 minutes, would cause the graphs in New Relic to drop to zero, rather than only graphing values sent to it at this poll interval. Also having a poll interval of 60 seconds that started at 25 seconds past the minute would cause the same problem at the start of the minute, but then be corrected when the metric was received. |
|
|
