-
Notifications
You must be signed in to change notification settings - Fork 83
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
Adaptive Load fake time source #491
Adaptive Load fake time source #491
Conversation
update from master
merge from upstream
merge from upstream
Signed-off-by: eric846 <[email protected]>
Signed-off-by: eric846 <[email protected]>
Signed-off-by: eric846 <[email protected]>
Signed-off-by: eric846 <[email protected]>
Signed-off-by: eric846 <[email protected]>
Signed-off-by: eric846 <[email protected]>
Signed-off-by: eric846 <[email protected]>
Signed-off-by: eric846 <[email protected]>
Signed-off-by: eric846 <[email protected]>
Signed-off-by: eric846 <[email protected]>
Signed-off-by: eric846 <[email protected]>
Signed-off-by: eric846 <[email protected]>
Signed-off-by: eric846 <[email protected]>
…double Signed-off-by: eric846 <[email protected]>
…variable_setter Signed-off-by: eric846 <[email protected]>
Signed-off-by: eric846 <[email protected]>
Signed-off-by: eric846 <[email protected]>
Signed-off-by: eric846 <[email protected]>
Signed-off-by: eric846 <[email protected]>
Signed-off-by: eric846 <[email protected]>
Signed-off-by: eric846 <[email protected]>
Signed-off-by: eric846 <[email protected]>
merge from upstream
…ent.Output turns out not to include the status Signed-off-by: eric846 <[email protected]>
…plugin names, log thresholds only once per session Signed-off-by: eric846 <[email protected]>
Signed-off-by: eric846 <[email protected]>
Signed-off-by: eric846 <[email protected]>
Signed-off-by: eric846 <[email protected]>
Signed-off-by: eric846 <[email protected]>
Signed-off-by: eric846 <[email protected]>
…e a reference, add file comments Signed-off-by: eric846 <[email protected]>
Signed-off-by: eric846 <[email protected]>
Signed-off-by: eric846 <[email protected]>
Signed-off-by: eric846 <[email protected]>
Signed-off-by: eric846 <[email protected]>
Signed-off-by: eric846 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -0,0 +1,17 @@ | |||
#include "test/adaptive_load/fake_time_source.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh sorry, I was looking at this in the context of ThreadSafeMonoticTimeSource
which might also be coming up, to see where this ended up in terms of physical location. I wonder if instead of placing this under test/adaptive_load/
, moving it into a shared directory like test/common/
would be possible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
('common' might not be the best name actually, but imho this ends up in a place where in terms of directory layout this and a new non-test time source would both end up with a consistent location, test/foo
and source/foo
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sense. Holding off on merging until Eric weighs in on where this should live.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to test/common
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think of test/common
?
- It's parallel to
source/common
- It definitely makes more sense than
test/adaptive_load
since a fake time source has nothing to do with adaptive load. - I don't want to add something directly in
test/
. test/test_common
looks like something we are forced to have by Envoy precedent but didn't make sense for other Nighthawk stuff?- It would be alongside tests for
ThreadSafeMonotonicTimeSource
and could be used from them if it made sense.
If we rearrange the whole test/
directory later, we can easily eliminate test/common
if there's an alternative.
Signed-off-by: eric846 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
A fake
Envoy::TimeSource
that ticks forward every time it is called, starting from 1970-01-01 00:00:00. OnlymonotonicTime()
is implemented.This simple autonomous behavior is needed because the adaptive load controller checks the clock many times automatically and can't be interrupted in the middle of the run to update the fake time.
Part 2 of splitting PR #483.