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.

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

[gcs] add missing templated log classes #8690

Merged
merged 1 commit into from
Jun 1, 2020
Merged

[gcs] add missing templated log classes #8690

merged 1 commit into from
Jun 1, 2020

Conversation

acxz
Copy link
Contributor

@acxz acxz commented May 31, 2020

Why are these changes needed?

Currently with the latest ray version, 0.8.5, it is not possible to build from source. (Tested on Arch Linux). This PR adds the relevant missing templated log classes to remove the undefined symbol errors.

Related issue number

Closes #853

Checks

  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/latest/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failure rates at https://ray-travis-tracker.herokuapp.com/.
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested (please justify below)

Above checks do not apply since this is a built PR, besides waiting for CI tests.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@acxz acxz changed the title add missing templated log classes [gcs] add missing templated log classes May 31, 2020
@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/26558/
Test PASSed.

@@ -870,6 +870,9 @@ template class Log<JobID, ErrorTableData>;
template class Log<ClientID, GcsNodeInfo>;
template class Log<JobID, JobTableData>;
template class Log<UniqueID, ProfileTableData>;
template class Log<ClientID, HeartbeatTableData>;
Copy link
Contributor

Choose a reason for hiding this comment

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

HeartbeatTable is inherit from Table<ClientID, HeartbeatTableData>, why add Log<ClientID, HeartbeatTableData>?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

HeartbeatTable may be inherited, but the associated Log<...> class needs to be instantiated since it is a template. If it is not then Log<ClientID, HeartbeatTableData> is not seen as a class further down resulting in the following error:

bazel-out/k8-opt/bin/_objs/gcs/subscription_executor.pic.o:subscription_executor.cc:function ray::gcs::SubscriptionExecutor<ray::ClientID, ray::rpc::HeartbeatTableData, ray::gcs::HeartbeatTable>::AsyncSubscribeAll(ray::ClientID const&, std::function<void (ray::ClientID const&, ray::rpc::HeartbeatTableData const&)> const&, std::function<void (ray::Status)> const&): error: undefined reference to 'ray::gcs::Log<ray::ClientID, ray::rpc::HeartbeatTableData>::Subscribe(ray::JobID const&, ray::ClientID const&, std::function<void (ray::gcs::RedisGcsClient*, ray::ClientID const&, std::vector<ray::rpc::HeartbeatTableData, std::allocator<ray::rpc::HeartbeatTableData> > const&)> const&, std::function<void (ray::gcs::RedisGcsClient*)> const&)'
collect2: error: ld returned 1 exit status

Just tested it with and without the line in question.

@simon-mo simon-mo merged commit 8b924a4 into ray-project:master Jun 1, 2020
@acxz acxz deleted the patch-1 branch June 1, 2020 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upload wheels that we build in Travis.
4 participants