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

rocksdb: revert the change introduced by dc41731 #10595

Merged
merged 1 commit into from Aug 5, 2016

Conversation

tchaikov
Copy link
Contributor

@tchaikov tchaikov commented Aug 5, 2016

dc41731 reverts the submodule reference of rocksdb, and point it to an
older version which fails to support ppc64le and AArch64.

Signed-off-by: Kefu Chai kchai@redhat.com

dc41731 reverts the submodule reference of rocksdb, and point it to an
older version which fails to support ppc64le and AArch64.

Signed-off-by: Kefu Chai <kchai@redhat.com>
@smithfarm
Copy link
Contributor

@tchaikov @dillaman I see jewel is on rocksdb fa98456 which is before 7ca731b so it doesn't support ppc64le and aarch64. . . Is this change backportable, do you think?

@tchaikov
Copy link
Contributor Author

tchaikov commented Aug 5, 2016

@smithfarm, as a part of #10438, we updated the rocksdb submodule and then reverted that change in #10060. in this change, we reverted the revert.

probably we can drop the change of rocksdb in #10060, and note it in "Conflicts", as that patch won't apply without #10438. and also AFAICT, no bug fix relies on the ppc64le and aarch64 support, so we don't need to backport #10438, right?

@smithfarm
Copy link
Contributor

smithfarm commented Aug 5, 2016

@tchaikov Correct, no jewel bugfixes need it. It's a feature request. @michelmno will be able to explain better, but my understanding is that facebook/rocksdb@7ca731b is needed for Ceph to build for ppc64le and aarch64, so there is an interest in updating the rocksdb submodule to that in the jewel release.

@michelmno
Copy link
Contributor

@smithfarm , @tchaikov, I still need the fix of facebook/rocksdb@7ca731b to be able to build ceph jewel branch for ppc64le.

@tchaikov
Copy link
Contributor Author

@michelmno i understand, but in general, we don't backport features.

@smithfarm
Copy link
Contributor

smithfarm commented Aug 22, 2016

@tchaikov I tend to see facebook/rocksdb@7ca731b as a build/ops fix. Could it be cherry-picked so we pick up just this one commit? (Like you did in #10750 for hammer)

@tchaikov
Copy link
Contributor Author

@smithfarm yeah. guess we need to have a tracker ticket for this fix then. @michaelndn could you file one for it?

@smithfarm
Copy link
Contributor

@michelmno I guess @tchaikov means that no feature like "ppc64(le) support" was contemplated or announced for jewel. The idea here is just to enable the build to complete on this architecture, not to support it.

@tchaikov
Copy link
Contributor Author

@smithfarm @michelmno exactly.

@michelmno
Copy link
Contributor

as suggested in #10595 (comment)
I just created new tracker issue http://tracker.ceph.com/issues/17092

@smithfarm
Copy link
Contributor

I just opened ceph/rocksdb#12 - once it is merged, we could bump the rocksdb sha1 in the jewel branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants