Introduce creation_time field into host description#13631
Introduce creation_time field into host description#13631mattklein123 merged 43 commits intoenvoyproxy:masterfrom
Conversation
bazel/envoy_internal.bzl
Outdated
There was a problem hiding this comment.
this will be cleaned up
|
Let me know when this is ready for review! /wait |
nezdolik
force-pushed
the
host-create_time
branch
from
d499c7b to
99e55a8
Compare
|
Managed to track down duplicated time source issue in failing tests (original_dst, logical_dns tests) with gdb and fixed it. Updated all tests under |
Signed-off-by: Kateryna Nezdolii <nezdolik@spotify.com>
nezdolik
force-pushed
the
host-create_time
branch
from
99e55a8 to
efd2b04
Compare
Signed-off-by: Kateryna Nezdolii <nezdolik@spotify.com>
nezdolik
force-pushed
the
host-create_time
branch
from
14c67b7 to
6d9006a
Compare
Signed-off-by: Kateryna Nezdolii <nezdolik@spotify.com>
Signed-off-by: Kateryna Nezdolii <nezdolik@spotify.com>
Signed-off-by: Kateryna Nezdolii <nezdolik@spotify.com>
Signed-off-by: Kateryna Nezdolii <nezdolik@gew1-atcdevbox-nezdolik-tws3.gew1.spotify.net>
|
my local machine clang formatter + buildifier formatted all bazel BUILD files (moved licence part at the top of file). Fixed that problem with running formatting in Docker |
Signed-off-by: Kateryna Nezdolii <nezdolik@spotify.com>
Signed-off-by: Kateryna Nezdolii <nezdolik@spotify.com>
Signed-off-by: Kateryna Nezdolii <nezdolik@spotify.com>
Signed-off-by: Kateryna Nezdolii <nezdolik@spotify.com>
Signed-off-by: Kateryna Nezdolii <nezdolik@spotify.com>
Signed-off-by: Kateryna Nezdolii <nezdolik@spotify.com>
|
@mattklein123 done |
mattklein123
left a comment
mattklein123
left a comment
There was a problem hiding this comment.
Thanks LGTM with small comments.
/wait
| HealthCheckHostMonitorPtr health_checker_; | ||
| std::atomic<uint32_t> priority_; | ||
| Network::TransportSocketFactory& socket_factory_; | ||
| MonotonicTime creation_time_; |
| redis_discovery_session_.resolve_timer_->enableTimer(std::chrono::milliseconds(0)); | ||
| })) { | ||
| })), | ||
| time_source_(dispatcher_.timeSource()) { |
There was a problem hiding this comment.
Can we have time_source_ in BaseDynamicClusterImpl or one of the other cluster base classes vs. having it in every derived class? That might be a bit simpler and cleaner?
Signed-off-by: Kateryna Nezdolii <nezdolik@spotify.com>
|
Not sure what is up with CI but pretty sure this is not going to pass format. /wait |
Signed-off-by: Kateryna Nezdolii <nezdolik@spotify.com>
Oh, i had git rebase in progress and did not notice it, so one commit did not got pushed to remote... |
Signed-off-by: Kateryna Nezdolii <nezdolik@spotify.com>
|
build pipeline on my dev box started failing with gcc, trying to fix it now and rerun build and tests. |
|
upgrading gcc,g++ to v 10 and purging bazel cache helped. I will keep pushing commits here to get faster feedback loop cycle. Am also running checks on devbox, but they are slower. |
|
@mattklein123 PTAL |
mattklein123
left a comment
mattklein123
left a comment
There was a problem hiding this comment.
Thanks! Feel free to fix the comment nit in your next change.
| virtual void priority(uint32_t) PURE; | ||
|
|
||
| /** | ||
| * @return timestamp in milliseconds of when host was created. |
There was a problem hiding this comment.
nit:
| * @return timestamp in milliseconds of when host was created. | |
| * @return timestamp when host was created. |
2 participants
Signed-off-by: Kateryna Nezdolii nezdolik@spotify.com
Description: Introducing "host creation time" field into host description. This change is prerequisite for supporting slow start in Envoy (#13176) and as it touches lots of code, it was decided to factor it out into dedicated PR.
Risk Level: Medium
Testing: In progress
Docs Changes: NA