-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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.
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
[xray] Fix GCS table prefixes #2065
[xray] Fix GCS table prefixes #2065
Conversation
@@ -297,8 +297,7 @@ class TaskReconstructionLog : public Log<TaskID, TaskReconstructionData> { | |||
TaskReconstructionLog(const std::shared_ptr<RedisContext> &context, | |||
AsyncGcsClient *client) | |||
: Log(context, client) { | |||
pubsub_channel_ = TablePubsub_ACTOR; |
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.
No need for pubsub_channel_ = TablePubsub_TASK_RECONSTRUCTION;
?
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, because pubsub isn't enabled for this table. There's a comment about this for the pubsub_channel_
declaration in the base class, but it's a bit hidden...I'll add a note to the class documentation.
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.
LGTM since this is an obvious change. Although I'm surprised there's no unit test that captured this.
Test PASSed. |
Test PASSed. |
* master: (22 commits) [xray] Fix bug in updating actor execution dependencies (ray-project#2064) [DataFrame] Refactor __delitem__ (ray-project#2080) [xray] Better error messaging when pulling from self. (ray-project#2068) Use source code in hash where possible (fix ray-project#2089) (ray-project#2090) Functions for flushing done tasks and evicted objects. (ray-project#2033) Fix compilation error for RAY_USE_NEW_GCS with latest clang. (ray-project#2086) [xray] Corrects Error Handling During Push and Pull. (ray-project#2059) [xray] Sophisticated task dependency management (ray-project#2035) Support calling positional arguments by keyword (fix ray-project#998) (ray-project#2081) [DataFrame] Improve performance of iteration methods (ray-project#2026) [DataFrame] Implement to_csv (ray-project#2014) [xray] Lineage cache only requests notifications about remote parent tasks (ray-project#2066) [rllib] Add magic methods for rollouts (ray-project#2024) [DataFrame] Allows DataFrame constructor to take in another DataFrame (ray-project#2072) Pin Pandas version for Travis to 0.22 (ray-project#2075) Fix python linting (ray-project#2076) [xray] Fix GCS table prefixes (ray-project#2065) Some tests for _submit API. (ray-project#2062) [rllib] Queue lib for python 2.7 (ray-project#2057) [autoscaler] Remove faulty assert that breaks during downscaling, pull configs from env (ray-project#2006) ...
* master: Pin Pandas version for Travis to 0.22 (ray-project#2075) Fix python linting (ray-project#2076) [xray] Fix GCS table prefixes (ray-project#2065) Some tests for _submit API. (ray-project#2062) [rllib] Queue lib for python 2.7 (ray-project#2057) [autoscaler] Remove faulty assert that breaks during downscaling, pull configs from env (ray-project#2006) [DataFrame] Refactor indexers and implement setitem (ray-project#2020) [rllib]Update bc/policy.py (ray-project#2012)
What do these changes do?
Each table in the GCS has a prefix and, if pubsub is enabled, a channel name. Tables in the GCS cannot share a prefix and can only share the "no pubsub" channel name. This fixes a bug where two tables had the same prefix.
Ideally, we would have some kind of static check to make sure that tables do not share the same names, but that is not included in this PR.