-
Notifications
You must be signed in to change notification settings - Fork 597
Replace CHECK() with CHECK_*() so that when a check is failing, more … #2847
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -245,7 +245,7 @@ class Client : public BaseClient { | |
__global_protobuf_pool_release__(m); | ||
return; | ||
} | ||
CHECK(m->IsInitialized()); | ||
CHECK(m->IsInitialized()) << "Protobuf not initialized"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't |
||
|
||
std::function<void()> cb = std::bind(method, _t, m); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -131,7 +131,7 @@ ZKClient::ZKClient(const std::string& hostportlist, EventLoop* eventLoop, | |
: eventLoop_(eventLoop), | ||
hostportlist_(hostportlist), | ||
client_global_watcher_cb_(std::move(global_watcher_cb)) { | ||
CHECK(client_global_watcher_cb_); | ||
CHECK(client_global_watcher_cb_) << "Client global watcher is not provided"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's better to explicitly say it's a callback |
||
Init(); | ||
} | ||
|
||
|
@@ -341,7 +341,7 @@ void ZKClient::ZkWatcherCb(VCallback<> cb) { | |
} | ||
|
||
void ZKClient::SendWatchEvent(const ZkWatchEvent& event) { | ||
CHECK(client_global_watcher_cb_); | ||
CHECK(client_global_watcher_cb_) << "Client global watcher is not provided"; | ||
piper_->ExecuteInEventLoop(std::bind(&RunWatchEventCb, client_global_watcher_cb_, event)); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -70,7 +70,7 @@ BoltInstance::~BoltInstance() { | |
} | ||
|
||
void BoltInstance::Start() { | ||
CHECK(!active_); | ||
CHECK(!active_) << "Bolt instance has been started already"; | ||
LOG(INFO) << "Starting bolt " << taskContext_->getThisComponentName(); | ||
bolt_->open(taskContext_->getConfig(), taskContext_, collector_); | ||
if (taskContext_->getConfig()->hasConfig(api::config::Config::TOPOLOGY_TICK_TUPLE_FREQ_SECS)) { | ||
|
@@ -86,13 +86,13 @@ void BoltInstance::Start() { | |
|
||
void BoltInstance::Activate() { | ||
LOG(INFO) << "Not doing anything in Bolt Activate"; | ||
CHECK(!active_); | ||
CHECK(!active_) << "Bolt instance has been started already"; | ||
active_ = true; | ||
} | ||
|
||
void BoltInstance::Deactivate() { | ||
LOG(INFO) << "Not doing anything in Bolt Dacctivate"; | ||
CHECK(active_); | ||
CHECK(!active_) << "Bolt instance is not active"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic changed:
|
||
active_ = false; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -74,31 +74,31 @@ HeronZKStateMgr::~HeronZKStateMgr() { | |
|
||
void HeronZKStateMgr::InitTree() { | ||
// Needs to be implemented | ||
CHECK(false); | ||
CHECK(false) << "HeronZKStateMgr::InitTree() is not implemented yet"; | ||
} | ||
|
||
void HeronZKStateMgr::SetTMasterLocationWatch(const std::string& topology_name, | ||
VCallback<> watcher) { | ||
CHECK(watcher); | ||
CHECK(!topology_name.empty()); | ||
CHECK(watcher) << "Watcher callback is missing in SetTMasterLocationWatch"; | ||
CHECK(!topology_name.empty()) << "Topology name is missing"; | ||
|
||
tmaster_location_watcher_info_ = new TMasterLocationWatchInfo(std::move(watcher), topology_name); | ||
SetTMasterLocationWatchInternal(); | ||
} | ||
|
||
void HeronZKStateMgr::SetMetricsCacheLocationWatch(const std::string& topology_name, | ||
VCallback<> watcher) { | ||
CHECK(watcher); | ||
CHECK(!topology_name.empty()); | ||
VCallback<> watcher) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. unrelated indent change |
||
CHECK(watcher) << "Watcher callback is missing in SetMetricsCacheLocationWatch"; | ||
CHECK(!topology_name.empty()) << "Topology name is missing"; | ||
|
||
metricscache_location_watcher_info_ = new TMasterLocationWatchInfo( | ||
std::move(watcher), topology_name); | ||
SetMetricsCacheLocationWatchInternal(); | ||
} | ||
|
||
void HeronZKStateMgr::SetPackingPlanWatch(const std::string& topology_name, VCallback<> watcher) { | ||
CHECK(watcher); | ||
CHECK(!topology_name.empty()); | ||
CHECK(watcher) << "Watcher callback is missing in SetPackingPlanWatch"; | ||
CHECK(!topology_name.empty()) << "Topology name is missing"; | ||
|
||
packing_plan_watcher_info_ = new TMasterLocationWatchInfo(std::move(watcher), topology_name); | ||
SetPackingPlanWatchInternal(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -131,7 +131,7 @@ StMgrClient* StMgrClientMgr::CreateClient(const sp_string& _other_stmgr_id, | |
bool StMgrClientMgr::SendTupleStreamMessage(sp_int32 _task_id, const sp_string& _stmgr_id, | ||
const proto::system::HeronTupleSet2& _msg) { | ||
auto iter = clients_.find(_stmgr_id); | ||
CHECK(iter != clients_.end()); | ||
CHECK(iter != clients_.end()) << "Stmgr " << _stmgr_id << " is not found in client list"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would prefer |
||
|
||
// Acquire the message | ||
proto::stmgr::TupleStreamMessage* out = nullptr; | ||
|
@@ -151,7 +151,7 @@ bool StMgrClientMgr::SendTupleStreamMessage(sp_int32 _task_id, const sp_string& | |
void StMgrClientMgr::SendDownstreamStatefulCheckpoint(const sp_string& _stmgr_id, | ||
proto::ckptmgr::DownstreamStatefulCheckpoint* _message) { | ||
auto iter = clients_.find(_stmgr_id); | ||
CHECK(iter != clients_.end()); | ||
CHECK(iter != clients_.end()) << "Stmgr " << _stmgr_id << " is not found in client list"; | ||
iter->second->SendDownstreamStatefulCheckpoint(_message); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -148,7 +148,8 @@ void StMgrServer::StartBackPressureClientCb(const sp_string& _other_stmgr_id) { | |
|
||
void StMgrServer::StopBackPressureClientCb(const sp_string& _other_stmgr_id) { | ||
CHECK(remote_ends_who_caused_back_pressure_.find(_other_stmgr_id) != | ||
remote_ends_who_caused_back_pressure_.end()); | ||
remote_ends_who_caused_back_pressure_.end()) | ||
<< "Stmgr " << _other_stmgr_id << " is not found in backpressure list"; | ||
remote_ends_who_caused_back_pressure_.erase(_other_stmgr_id); | ||
|
||
if (!stmgr_->DidAnnounceBackPressure()) { | ||
|
@@ -175,7 +176,8 @@ void StMgrServer::HandleStartBackPressureMessage(Connection* _conn, | |
return; | ||
} | ||
auto iter = rstmgrs_.find(_conn); | ||
CHECK(iter != rstmgrs_.end()); | ||
CHECK(iter != rstmgrs_.end()) | ||
<< "Connection " << _conn << " is not found in remote stmgr list when starting backpressure"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure how |
||
sp_string stmgr_id = iter->second; | ||
stmgrs_who_announced_back_pressure_.insert(stmgr_id); | ||
|
||
|
@@ -196,7 +198,8 @@ void StMgrServer::HandleStopBackPressureMessage(Connection* _conn, | |
return; | ||
} | ||
auto iter = rstmgrs_.find(_conn); | ||
CHECK(iter != rstmgrs_.end()); | ||
CHECK(iter != rstmgrs_.end()) | ||
<< "Connection " << _conn << " is not found in remote stmgr list when stopping backpressure"; | ||
sp_string stmgr_id = iter->second; | ||
// Did we receive a start back pressure message from this stmgr to | ||
// begin with? We could have been dead at the time of the announcement | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -282,7 +282,7 @@ void StMgr::FetchMetricsCacheLocation() { | |
} | ||
|
||
void StMgr::StartStmgrServer() { | ||
CHECK(!stmgr_server_); | ||
CHECK(!stmgr_server_) << "Stmgr server is already running"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
LOG(INFO) << "Creating StmgrServer"; | ||
NetworkOptions sops; | ||
sops.set_host(IpUtils::getHostName()); | ||
|
@@ -302,7 +302,7 @@ void StMgr::StartStmgrServer() { | |
} | ||
|
||
void StMgr::StartInstanceServer() { | ||
CHECK(!instance_server_); | ||
CHECK(!instance_server_) << "InstanceServer server is already running"; | ||
LOG(INFO) << "Creating InstanceServer"; | ||
NetworkOptions sops; | ||
sops.set_host(IpUtils::getHostName()); | ||
|
@@ -348,7 +348,7 @@ void StMgr::CreateCheckpointMgrClient() { | |
} | ||
|
||
void StMgr::CreateTMasterClient(proto::tmaster::TMasterLocation* tmasterLocation) { | ||
CHECK(!tmaster_client_); | ||
CHECK(!tmaster_client_) << "TMaster client has already existed"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
LOG(INFO) << "Creating Tmaster Client at " << tmasterLocation->host() << ":" | ||
<< tmasterLocation->master_port(); | ||
NetworkOptions master_options; | ||
|
@@ -384,7 +384,7 @@ void StMgr::CreateTMasterClient(proto::tmaster::TMasterLocation* tmasterLocation | |
} | ||
|
||
void StMgr::CreateTupleCache() { | ||
CHECK(!tuple_cache_); | ||
CHECK(!tuple_cache_) << "Tuple cache has already existed"; | ||
LOG(INFO) << "Creating tuple cache "; | ||
sp_uint32 drain_threshold_bytes_ = | ||
config::HeronInternalsConfigReader::Instance()->GetHeronStreammgrCacheDrainSizeMb() * 1_MB; | ||
|
@@ -999,7 +999,7 @@ void StMgr::InitiateStatefulCheckpoint(sp_string _checkpoint_id) { | |
void StMgr::HandleStoreInstanceStateCheckpoint( | ||
const proto::ckptmgr::InstanceStateCheckpoint& _message, | ||
const proto::system::Instance& _instance) { | ||
CHECK(stateful_restorer_); | ||
CHECK(stateful_restorer_) << "Stateful restorer doesn't exist when storing checkpoint"; | ||
int32_t task_id = _instance.info().task_id(); | ||
LOG(INFO) << "Got a checkpoint state message from " << task_id | ||
<< " for checkpoint " << _message.checkpoint_id(); | ||
|
@@ -1072,7 +1072,7 @@ void StMgr::HandleDownStreamStatefulCheckpoint( | |
void StMgr::RestoreTopologyState(sp_string _checkpoint_id, sp_int64 _restore_txid) { | ||
LOG(INFO) << "Got a Restore Topology State message from Tmaster for checkpoint " | ||
<< _checkpoint_id << " and txid " << _restore_txid; | ||
CHECK(stateful_restorer_); | ||
CHECK(stateful_restorer_) << "Stateful restorer doesn't exist when restoring state"; | ||
|
||
// Start the restore process | ||
std::unordered_set<sp_int32> local_taskids; | ||
|
@@ -1085,7 +1085,7 @@ void StMgr::RestoreTopologyState(sp_string _checkpoint_id, sp_int64 _restore_txi | |
void StMgr::StartStatefulProcessing(sp_string _checkpoint_id) { | ||
LOG(INFO) << "Received StartProcessing message from tmaster for " | ||
<< _checkpoint_id; | ||
CHECK(stateful_restorer_); | ||
CHECK(stateful_restorer_) << "Stateful restorer doesn't exist when starting process"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe |
||
if (stateful_restorer_->InProgress()) { | ||
LOG(FATAL) << "StartProcessing received from Tmaster for " | ||
<< _checkpoint_id << " when we are still in Restore"; | ||
|
@@ -1099,7 +1099,7 @@ void StMgr::HandleRestoreInstanceStateResponse(sp_int32 _task_id, | |
// If we are stateful topology, we might want to see how the restore went | ||
// and if it was successful and all other local instances have recovered | ||
// send back a success response to tmaster saying that we have recovered | ||
CHECK(stateful_restorer_); | ||
CHECK(stateful_restorer_) << "Stateful restorer doesn't exist when querying restore state"; | ||
stateful_restorer_->HandleInstanceRestoredState(_task_id, _status.status(), _checkpoint_id); | ||
} | ||
|
||
|
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.
I think it's either unrelated change or need to be rebased
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.
Yeah. It is an unrelated change and has been merged in a different PR.