Skip to content

Commit

Permalink
[core] Deflakey gcs server rpc test (#31919)
Browse files Browse the repository at this point in the history
The test failed because in the current ray redis client there is a global variable using io context. and the io context is freed during destruction and when destruct the global variable it'll cause issues.

The current redis client is aweful and ugly. Since we'll move to redis plus plus, and this client doesn't cause other issues, I'll just fix the test case.
  • Loading branch information
fishbone authored Jan 25, 2023
1 parent 6aec692 commit b7d6f2f
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 0 deletions.
1 change: 1 addition & 0 deletions src/ray/gcs/gcs_server/test/gcs_server_rpc_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ class GcsServerTest : public ::testing::Test {
gcs_server_->Stop();
thread_io_service_->join();
gcs_server_.reset();
ray::gcs::RedisCallbackManager::instance().Clear();
}

bool AddJob(const rpc::AddJobRequest &request) {
Expand Down
5 changes: 5 additions & 0 deletions src/ray/gcs/redis_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,11 @@ std::shared_ptr<RedisCallbackManager::CallbackItem> RedisCallbackManager::GetCal
return it->second;
}

void RedisCallbackManager::Clear() {
std::lock_guard<std::mutex> lock(mutex_);
callback_items_.clear();
}

void RedisCallbackManager::RemoveCallback(int64_t callback_index) {
std::lock_guard<std::mutex> lock(mutex_);
callback_items_.erase(callback_index);
Expand Down
3 changes: 3 additions & 0 deletions src/ray/gcs/redis_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,9 @@ class RedisCallbackManager {
/// Get a callback.
std::shared_ptr<CallbackItem> GetCallback(int64_t callback_index) const;

/// Clear all callbacks.
void Clear();

private:
RedisCallbackManager() : num_callbacks_(0){};

Expand Down

0 comments on commit b7d6f2f

Please sign in to comment.