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.

Sign up for GitHub

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

Jump to bottom

test: track simulated timers per dispatcher to simplify thread interactions and locking. #12609

Merged
merged 12 commits into from
Aug 26, 2020
Merged

test: track simulated timers per dispatcher to simplify thread interactions and locking. #12609

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
ff46275
Track simulated timers by dispatcher in order to simplify locking and…
antoniovicente Aug 11, 2020
9062235
Wait until idle when deleting simulated timers from outside the dispa…
antoniovicente Aug 12, 2020
810b4ae
comment
antoniovicente Aug 12, 2020
75972c2
cleanup
antoniovicente Aug 12, 2020
2dd0e2e
Merge remote-tracking branch 'upstream/master' into simulated_time_pe…
antoniovicente Aug 19, 2020
8aadbef
fix test failures and address review comments in preparation for remo…
antoniovicente Aug 20, 2020
5e2ded6
format
antoniovicente Aug 21, 2020
16a7656
add comment
antoniovicente Aug 21, 2020
8d9bd42
fix typo
antoniovicente Aug 24, 2020
4a2c7b8
update spelling dictionary
antoniovicente Aug 24, 2020
9b4c771
adjust implementation to avoid double negative
antoniovicente Aug 25, 2020
30b8695
revert unnecessary changes to gradient_controller_test
antoniovicente Aug 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion test/extensions/quic_listeners/quiche/active_quic_listener_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,8 @@ class ActiveQuicListenerTest : public QuicMultiVersionTest {
auto proof_source = std::make_unique<TestProofSource>();
filter_chain_ = &proof_source->filterChain();
crypto_config_peer.ResetProofSource(std::move(proof_source));
simulated_time_system_.advanceTimeWait(std::chrono::milliseconds(100));
simulated_time_system_.advanceTimeAsync(std::chrono::milliseconds(100));
dispatcher_->run(Event::Dispatcher::RunType::NonBlock);
}

Network::ActiveUdpListenerFactoryPtr createQuicListenerFactory(const std::string& yaml) {
Expand Down
3 changes: 2 additions & 1 deletion test/extensions/quic_listeners/quiche/envoy_quic_dispatcher_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ class EnvoyQuicDispatcherTest : public QuicMultiVersionTest,

void SetUp() override {
// Advance time a bit because QuicTime regards 0 as uninitialized timestamp.
time_system_.advanceTimeWait(std::chrono::milliseconds(100));
time_system_.advanceTimeAsync(std::chrono::milliseconds(100));
dispatcher_->run(Event::Dispatcher::RunType::NonBlock);
EXPECT_CALL(listener_config_, perConnectionBufferLimitBytes())
.WillRepeatedly(Return(1024 * 1024));
}
Expand Down
12 changes: 8 additions & 4 deletions test/extensions/quic_listeners/quiche/envoy_quic_server_session_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,8 @@ class EnvoyQuicServerSessionTest : public testing::TestWithParam<bool> {

// Advance time and trigger update of Dispatcher::approximateMonotonicTime()
// because zero QuicTime is considered uninitialized.
time_system_.advanceTimeWait(std::chrono::milliseconds(1));
time_system_.advanceTimeAsync(std::chrono::milliseconds(1));
dispatcher_->run(Event::Dispatcher::RunType::NonBlock);
connection_helper_.GetClock()->Now();

ON_CALL(writer_, WritePacket(_, _, _, _, _))
Expand Down Expand Up @@ -669,7 +670,8 @@ TEST_P(EnvoyQuicServerSessionTest, FlushAndWaitForCloseWithTimeout) {
EXPECT_EQ(Network::Connection::State::Open, envoy_quic_session_.state());
// Unblocking the stream shouldn't close the connection as it should be
// delayed.
time_system_.advanceTimeWait(std::chrono::milliseconds(10));
time_system_.advanceTimeAsync(std::chrono::milliseconds(10));
dispatcher_->run(Event::Dispatcher::RunType::NonBlock);
envoy_quic_session_.OnCanWrite();
// delay close alarm should have been rescheduled.
time_system_.advanceTimeAsync(std::chrono::milliseconds(90));
Expand Down Expand Up @@ -700,7 +702,8 @@ TEST_P(EnvoyQuicServerSessionTest, FlusWriteTransitToFlushWriteWithDelay) {
envoy_quic_session_.close(Network::ConnectionCloseType::FlushWrite);
EXPECT_EQ(Network::Connection::State::Open, envoy_quic_session_.state());

time_system_.advanceTimeWait(std::chrono::milliseconds(10));
time_system_.advanceTimeAsync(std::chrono::milliseconds(10));
dispatcher_->run(Event::Dispatcher::RunType::NonBlock);
// The closing behavior should be changed.
envoy_quic_session_.close(Network::ConnectionCloseType::FlushWriteAndDelay);
// Unblocking the stream shouldn't close the connection as it should be
Expand Down Expand Up @@ -732,7 +735,8 @@ TEST_P(EnvoyQuicServerSessionTest, FlushAndWaitForCloseWithNoPendingData) {

// Advance the time a bit and try to close again. The delay close timer
// shouldn't be rescheduled by this call.
time_system_.advanceTimeWait(std::chrono::milliseconds(10));
time_system_.advanceTimeAsync(std::chrono::milliseconds(10));
dispatcher_->run(Event::Dispatcher::RunType::NonBlock);
envoy_quic_session_.close(Network::ConnectionCloseType::FlushWriteAndDelay);
EXPECT_EQ(Network::Connection::State::Open, envoy_quic_session_.state());

Expand Down
1 change: 1 addition & 0 deletions test/server/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ envoy_cc_test(

envoy_cc_test(
name = "guarddog_impl_test",
size = "small",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Explanation of this change:
Before other changes in this PR this test occasionally times out due to flaky alarm behavior as seen in #12638

This test usually runs in about 5s to 10s in standard confirgurations, and 20 seconds under clang-tsan. It may make sense to reduce the configured timeout for this test to small which has an associated timeout of 60 secs in order to get faster feedback when this test fails due to timeout.

srcs = ["guarddog_impl_test.cc"],
tags = ["flaky_on_windows"],
deps = [
Expand Down
Loading