ClearcutV1ShippingManager doesn't need SendRetryer
CB-141 #done
ClearcutV1ShippingManager now no longer takes SendRetryer as an
argument, now only the LegacyShippingManager takes it.
Change-Id: I539990ddd586f787b8b6854fbfc8d49c12fb3b64
diff --git a/encoder/shipping_dispatcher_test.cc b/encoder/shipping_dispatcher_test.cc
index 7f77af5..d4def22 100644
--- a/encoder/shipping_dispatcher_test.cc
+++ b/encoder/shipping_dispatcher_test.cc
@@ -42,8 +42,6 @@
// 40 bytes, the worker thread will attempt to send Envelopes that contain
// exactly 5, 40-byte Observations. (4 * 40 < 170 and 6 * 40 > 200 ).
const size_t kMinEnvelopeSendSize = 170;
-const std::chrono::seconds kInitialRpcDeadline(10);
-const std::chrono::seconds kDeadlinePerSendAttempt(60);
const std::chrono::seconds kMaxSeconds = ShippingManager::kMaxSeconds;
// Returns a ProjectContext obtained by parsing the configuration specified
@@ -86,51 +84,6 @@
SystemProfile system_profile_;
};
-struct FakeSendRetryer : public SendRetryerInterface {
- FakeSendRetryer() : SendRetryerInterface() {}
-
- grpc::Status SendToShuffler(
- std::chrono::seconds initial_rpc_deadline,
- std::chrono::seconds overerall_deadline,
- send_retryer::CancelHandle* cancel_handle,
- const EncryptedMessage& encrypted_message) override {
- // Decrypt encrypted_message. (No actual decryption is involved since
- // we used the NONE encryption scheme.)
- util::MessageDecrypter decrypter("");
- Envelope recovered_envelope;
- EXPECT_TRUE(
- decrypter.DecryptMessage(encrypted_message, &recovered_envelope));
- EXPECT_EQ(1, recovered_envelope.batch_size());
- FakeSystemData::CheckSystemProfile(recovered_envelope);
-
- std::unique_lock<std::mutex> lock(mutex);
- send_call_count++;
- observation_count +=
- recovered_envelope.batch(0).encrypted_observation_size();
- // We grab the return value before we block. This allows the test
- // thread to wait for us to block, then change the value of
- // status_to_return for the *next* send without changing it for
- // the currently blocking send.
- grpc::Status status = status_to_return;
- if (should_block) {
- is_blocking = true;
- send_is_blocking_notifier.notify_all();
- send_can_exit_notifier.wait(lock, [this] { return !should_block; });
- is_blocking = false;
- }
- return status;
- }
-
- std::mutex mutex;
- bool should_block = false;
- std::condition_variable send_can_exit_notifier;
- bool is_blocking = false;
- std::condition_variable send_is_blocking_notifier;
- grpc::Status status_to_return = grpc::Status::OK;
- int send_call_count = 0;
- int observation_count = 0;
-};
-
class FakeHTTPClient : public clearcut::HTTPClient {
public:
explicit FakeHTTPClient(std::set<uint32_t>* seen_event_codes)
@@ -162,18 +115,14 @@
class FakeShippingManager : public ShippingManager {
public:
FakeShippingManager(std::chrono::seconds schedule_interval,
- std::chrono::seconds min_interval,
- SendRetryerInterface* send_retryer)
+ std::chrono::seconds min_interval)
: ShippingManager(
ShippingManager::SizeParams(kMaxBytesPerObservation,
kMaxBytesPerEnvelope, kMaxBytesTotal,
kMinEnvelopeSendSize),
ShippingManager::ScheduleParams(schedule_interval, min_interval),
ShippingManager::EnvelopeMakerParams("", EncryptedMessage::NONE, "",
- EncryptedMessage::NONE),
- ShippingManager::SendRetryerParams(kInitialRpcDeadline,
- kDeadlinePerSendAttempt),
- send_retryer),
+ EncryptedMessage::NONE)),
envelopes_sent_(0) {}
void SendEnvelopeToBackend(
@@ -205,15 +154,12 @@
protected:
void Init(std::chrono::seconds schedule_interval,
std::chrono::seconds min_interval) {
- send_retryer_.reset(new FakeSendRetryer());
shipping_dispatcher_->Register(
ObservationMetadata::LEGACY_BACKEND,
- std::make_unique<FakeShippingManager>(schedule_interval, min_interval,
- send_retryer_.get()));
+ std::make_unique<FakeShippingManager>(schedule_interval, min_interval));
shipping_dispatcher_->Register(
ObservationMetadata::V1_BACKEND,
- std::make_unique<FakeShippingManager>(schedule_interval, min_interval,
- send_retryer_.get()));
+ std::make_unique<FakeShippingManager>(schedule_interval, min_interval));
shipping_dispatcher_->Start();
}
@@ -233,7 +179,6 @@
FakeSystemData system_data_;
std::set<uint32_t> seen_clearcut_event_codes_;
- std::unique_ptr<FakeSendRetryer> send_retryer_;
std::unique_ptr<ShippingDispatcher> shipping_dispatcher_;
std::shared_ptr<ProjectContext> project_;
Encoder encoder_;
diff --git a/encoder/shipping_manager.cc b/encoder/shipping_manager.cc
index f1d0ab6..d802e82 100644
--- a/encoder/shipping_manager.cc
+++ b/encoder/shipping_manager.cc
@@ -26,20 +26,15 @@
ShippingManager::ShippingManager(
const SizeParams& size_params, const ScheduleParams& schedule_params,
- const EnvelopeMakerParams& envelope_maker_params,
- const SendRetryerParams send_retryer_params,
- SendRetryerInterface* send_retryer)
+ const EnvelopeMakerParams& envelope_maker_params)
: size_params_(size_params),
envelope_send_threshold_size_(
size_t(0.6 * size_params.max_bytes_per_envelope_)),
total_bytes_send_threshold_(size_t(0.6 * size_params.max_bytes_total_)),
schedule_params_(schedule_params),
envelope_maker_params_(envelope_maker_params),
- send_retryer_params_(send_retryer_params),
- send_retryer_(send_retryer),
next_scheduled_send_time_(std::chrono::system_clock::now() +
schedule_params_.schedule_interval_) {
- CHECK(send_retryer);
_mutex_protected_fields_do_not_access_directly_.active_envelope_maker.reset(
new EnvelopeMaker(envelope_maker_params.analyzer_public_key_pem_,
envelope_maker_params.analyzer_scheme_,
@@ -92,7 +87,7 @@
case EnvelopeMaker::kOk:
VLOG(4) << "ShippingManager::AddObservation: OK";
// Set idle_ false because any thread that invokes WaitUntilIdle() after
- // this should wait until the Observatoin just added has been sent.
+ // this should wait until the Observation just added has been sent.
locked->fields->idle = false;
break;
@@ -371,8 +366,11 @@
const EnvelopeMakerParams& envelope_maker_params,
const SendRetryerParams send_retryer_params,
SendRetryerInterface* send_retryer)
- : ShippingManager(size_params, scheduling_params, envelope_maker_params,
- send_retryer_params, send_retryer) {}
+ : ShippingManager(size_params, scheduling_params, envelope_maker_params),
+ send_retryer_params_(send_retryer_params),
+ send_retryer_(send_retryer) {
+ CHECK(send_retryer_);
+}
void LegacyShippingManager::SendEnvelopeToBackend(
std::unique_ptr<EnvelopeMaker> envelope_to_send,
@@ -411,11 +409,8 @@
ClearcutV1ShippingManager::ClearcutV1ShippingManager(
const SizeParams& size_params, const ScheduleParams& scheduling_params,
const EnvelopeMakerParams& envelope_maker_params,
- const SendRetryerParams send_retryer_params,
- SendRetryerInterface* send_retryer,
std::unique_ptr<clearcut::ClearcutUploader> clearcut)
- : ShippingManager(size_params, scheduling_params, envelope_maker_params,
- send_retryer_params, send_retryer),
+ : ShippingManager(size_params, scheduling_params, envelope_maker_params),
clearcut_(std::move(clearcut)) {}
void ClearcutV1ShippingManager::SendEnvelopeToBackend(
diff --git a/encoder/shipping_manager.h b/encoder/shipping_manager.h
index d5dc30f..339bbfe 100644
--- a/encoder/shipping_manager.h
+++ b/encoder/shipping_manager.h
@@ -187,18 +187,9 @@
//
// envelope_maker_params: Used when the ShippingManager needs to construct
// and EnvelopeMaker.
- //
- // send_retryer_params: Used when the ShippingManager needs to invoke
- // SendRetryer::SendToShuffler().
- //
- // send_retryer: The instance of |SendRetryerInterface| encapsulated by
- // this ShippingManager. ShippingManager does not take ownership of
- // send_retryer which must outlive ShippingManager.
ShippingManager(const SizeParams& size_params,
const ScheduleParams& scheduling_params,
- const EnvelopeMakerParams& envelope_maker_params,
- const SendRetryerParams send_retryer_params,
- SendRetryerInterface* send_retryer);
+ const EnvelopeMakerParams& envelope_maker_params);
// The destructor will stop the worker thread and wait for it to stop
// before exiting.
@@ -328,11 +319,6 @@
const ScheduleParams schedule_params_;
const EnvelopeMakerParams envelope_maker_params_;
- protected:
- const SendRetryerParams send_retryer_params_;
-
- SendRetryerInterface* send_retryer_; // not owned
-
private:
// Variables accessed only by the worker thread. These are not
// protected by a mutex.
@@ -453,6 +439,12 @@
class LegacyShippingManager : public ShippingManager {
public:
+ // send_retryer_params: Used when the ShippingManager needs to invoke
+ // SendRetryer::SendToShuffler().
+ //
+ // send_retryer: The instance of |SendRetryerInterface| encapsulated by
+ // this ShippingManager. ShippingManager does not take ownership of
+ // send_retryer which must outlive ShippingManager.
LegacyShippingManager(const SizeParams& size_params,
const ScheduleParams& scheduling_params,
const EnvelopeMakerParams& envelope_maker_params,
@@ -460,6 +452,10 @@
SendRetryerInterface* send_retryer);
private:
+ const SendRetryerParams send_retryer_params_;
+
+ SendRetryerInterface* send_retryer_; // not owned
+
void SendEnvelopeToBackend(
std::unique_ptr<EnvelopeMaker> envelope_to_send,
std::deque<std::unique_ptr<EnvelopeMaker>>* envelopes_that_failed);
@@ -470,8 +466,6 @@
ClearcutV1ShippingManager(
const SizeParams& size_params, const ScheduleParams& scheduling_params,
const EnvelopeMakerParams& envelope_maker_params,
- const SendRetryerParams send_retryer_params,
- SendRetryerInterface* send_retryer,
std::unique_ptr<::clearcut::ClearcutUploader> clearcut);
private:
diff --git a/encoder/shipping_manager_test.cc b/encoder/shipping_manager_test.cc
index e3174d0..8d9a0e1 100644
--- a/encoder/shipping_manager_test.cc
+++ b/encoder/shipping_manager_test.cc
@@ -195,7 +195,6 @@
} else {
shipping_manager_.reset(new ClearcutV1ShippingManager(
size_params, schedule_params, envelope_maker_params,
- send_retryer_params, send_retryer_.get(),
std::make_unique<clearcut::ClearcutUploader>(
"https://test.com",
std::make_unique<FakeHTTPClient>(&seen_clearcut_event_codes_))));
diff --git a/tools/test_app/test_app.cc b/tools/test_app/test_app.cc
index a3fff09..a157865 100644
--- a/tools/test_app/test_app.cc
+++ b/tools/test_app/test_app.cc
@@ -577,7 +577,6 @@
ObservationMetadata::V1_BACKEND,
std::make_unique<ClearcutV1ShippingManager>(
size_params, schedule_params, envelope_maker_params,
- send_retryer_params, send_retryer_.get(),
std::make_unique<clearcut::ClearcutUploader>(
FLAGS_clearcut_endpoint,
std::make_unique<util::clearcut::CurlHTTPClient>())));
