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.

Sign up for GitHub

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

Jump to bottom

Add s390x builds to Travis #6168

Closed
wants to merge 6 commits into from
Closed

Add s390x builds to Travis #6168

wants to merge 6 commits into from

Conversation

adamretter
Copy link
Collaborator

Highly experimental ;-)

@adamretter adamretter force-pushed the feature/travis-s390x branch 5 times, most recently from 9a09c19 to 2b3beae Compare December 21, 2019 08:31
@adamretter adamretter mentioned this pull request Dec 21, 2019
Open
@sangitanalkar sangitanalkar mentioned this pull request Jan 14, 2020
Merged
@sangitanalkar
Copy link

sangitanalkar commented Jan 14, 2020 •
edited
Loading

Hi, any updates on the PR if the changes can be merged?

@adamretter adamretter force-pushed the feature/travis-s390x branch from 2b3beae to 13f16d9 Compare January 14, 2020 12:00
@adamretter
Copy link
Collaborator Author

@sangitanalkar No progress yet, there are a couple of issues that need to be investigated including - #6236

In addition whilst we have CI for s390x. We do not have access to an s390x system for building releases on, without that we would be unable to release binaries for that platform.

@sangitanalkar
Copy link

@adamretter we could provide the s390x system, please let us know the configuration that is needed.

@adamretter
Copy link
Collaborator Author

@sangitanalkar could you contact me directly by email to discuss.

@adamretter adamretter force-pushed the feature/travis-s390x branch from 13f16d9 to f90e20c Compare March 11, 2020 18:40
@adamretter
Copy link
Collaborator Author

I also emailed @edelsohn about getting access to a community development system.

@edelsohn
Copy link

LinuxONE Community Cloud

@adamretter adamretter force-pushed the feature/travis-s390x branch from f90e20c to 52c7c28 Compare March 11, 2020 22:24
@adamretter adamretter requested a review from pdillinger March 11, 2020 22:51
@adamretter adamretter force-pushed the feature/travis-s390x branch 2 times, most recently from a7a24d1 to af86eb8 Compare March 12, 2020 16:18
pdillinger
pdillinger reviewed Mar 12, 2020
View reviewed changes
.travis.yml Outdated
@@ -25,6 +26,7 @@ addons:
- liblzma-dev # xv
- libzstd-dev
- zlib1g-dev
- libssl-dev # needed for CMake to download deps
Copy link
Contributor

Choose a reason for hiding this comment

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

This probably isn't needed now that we have pre-built binary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will remove it

.travis.yml Show resolved Hide resolved
@adamretter adamretter force-pushed the feature/travis-s390x branch from fc62452 to 4a6e002 Compare March 12, 2020 22:21
pdillinger added a commit to pdillinger/rocksdb that referenced this pull request Mar 13, 2020
@pdillinger
Update folly/lang/Align.h + backport to C++11
ebc5edf 
Summary: For s390x support, some updates in newer version of Align.h are
needed. Upgrading just that file as best we can, with one addition to
Portability.h and tweaking new code in Align.h to use C++11 only (no
non-trivial constexpr functions).

Test Plan: CI, further work in PR facebook#6168
@pdillinger pdillinger mentioned this pull request Mar 13, 2020
Closed
facebook-github-bot pushed a commit that referenced this pull request Mar 17, 2020
@pdillinger @facebook-github-bot
Update folly/lang/Align.h (backport to C++11) (#6534)
6c595f0 
Summary:
For s390x support, some updates in newer version of Align.h are
needed. Upgrading just that file as best we can, with one addition to
Portability.h and tweaking new code in Align.h to use C++11 only (no
non-trivial constexpr functions).
Pull Request resolved: #6534

Test Plan: CI, further work in PR #6168

Differential Revision: D20445942

Pulled By: pdillinger

fbshipit-source-id: 0cef3c367463c71f3123d12cdf287c573af5e342
@adamretter adamretter force-pushed the feature/travis-s390x branch from 4a6e002 to 5a9fe9e Compare March 24, 2020 09:46
@adamretter adamretter force-pushed the feature/travis-s390x branch from 8bd055a to 9d330d6 Compare April 29, 2020 20:52
@adamretter adamretter force-pushed the feature/travis-s390x branch from b774b6d to e9569b7 Compare October 14, 2020 17:56
@adamretter adamretter mentioned this pull request Oct 19, 2020
Closed
@adamretter adamretter force-pushed the feature/travis-s390x branch from e9569b7 to 20462ce Compare January 9, 2021 11:41
@jonathan-albrecht-ibm
Copy link
Contributor

Hi @adamretter, I'd like to work on adding s390x support to rocksdb. I'm part of the same group that was asking about s390x support in #8642 but I'm just looking at the latest rocksdb.

I've starting with a rebased fork of adamretter:feature/travis-s390x and added a few fixes for recent changes. I'm down to three failing tests:

  1. Full/FullBloomTest.OptimizeForMemory/0
  2. DBPropertiesTest.AggregatedTableProperties
  3. DBPropertiesTest.AggregatedTableProperties

and all failures seem related to bloom filters, eg:

...
[ RUN      ] Full/FullBloomTest.OptimizeForMemory/0
/home/jalbrecht/src/rocksdb/util/bloom_test.cc:555: Failure
Expected: (total_fp_rate / double{nfilters}) >= (0.008), actual: 0.007808 vs 0.008
...
[ RUN      ] DBPropertiesTest.AggregatedTableProperties                                                                     /home/jalbrecht/src/rocksdb/db/db_properties_test.cc:208: Failure                                                           Expected: (static_cast<double>(dbl_a - dbl_b) / (dbl_a + dbl_b)) < (bias), actual: 0.167604 vs 0.15  
[ RUN      ] DBPropertiesTest.AggregatedTablePropertiesAtLevel                                                              /home/jalbrecht/src/rocksdb/db/db_properties_test.cc:208: Failure
Expected: (static_cast<double>(dbl_a - dbl_b) / (dbl_a + dbl_b)) < (bias), actual: 0.518044 vs 0.5    
...

I'm not sure where to start with these. I posted this to the facebook RocksDB Developers group but didn't get a response so not sure I picked the best forum. Any suggestions on where or who to ask would be great.

I noticed all of theses tests set a seed for the random number generator so the same sequence is always used. Just a wild guess but I'm wondering if the threshold values.(0.008, 0.15, 0.5) are not really hard thresholds and if the errors might not indicate a real problem on s390x.

Thanks! Happy to provide any more info or testing as needed.

@jonathan-albrecht-ibm jonathan-albrecht-ibm mentioned this pull request Sep 27, 2021
Closed
facebook-github-bot pushed a commit that referenced this pull request Oct 22, 2021
@jonathan-albrecht-ibm @facebook-github-bot
Add support for building on s390x platform (#8962)
e970248 
Summary:
This PR adds support for building on s390x including updating travis CI. It uses the previous work in #6168 and adds some more changes to get all current tests (make check and jni tests) to pass. The tests were run with snappy, lz4, bzip2 and zstd all compiled in.

There are a few pieces still needed to get the travis build working that I don't think I can do. adamretter is this something you could help with?

1. A prebuilt https://rocksdb-deps.s3-us-west-2.amazonaws.com/cmake/cmake-3.14.5-Linux-s390x.deb package
2. A https://hub.docker.com/r/evolvedbinary/rocksjava s390x image

Not sure if there is more required for travis. Happy to help in any way I can.

Pull Request resolved: #8962

Reviewed By: mrambacher

Differential Revision: D31802198

Pulled By: pdillinger

fbshipit-source-id: 683511466fa6b505f85ba5a9964a268c6151f0c2
@adamretter
Copy link
Collaborator Author

@jonathan-albrecht-ibm Sorry for the delay. Would you be able to invite me again to your https://github.com/jonathan-albrecht-ibm/cmake-3.14.5-Linux-s390x-deb repository please?

@jonathan-albrecht-ibm
Copy link
Contributor

@jonathan-albrecht-ibm Sorry for the delay. Would you be able to invite me again to your https://github.com/jonathan-albrecht-ibm/cmake-3.14.5-Linux-s390x-deb repository please?

np @adamretter I've resent the invite.

@adamretter
Copy link
Collaborator Author

@jonathan-albrecht-ibm Got it. Thanks :-)

@adamretter
Copy link
Collaborator Author

@jonathan-albrecht-ibm Can this now be closed in favour of your work in #8962 ?

I have one question about your PR (#8962)... Why the arch is set to z196 as opposed to z10, I have to admit that I am not clear about the numbering of the Z architecture. If I recall, I chose z10 as it was a reasonably old one that was also supported on Travis CI - so I figured it would give us the largest possible backwards/forwards compatibility.

@jonathan-albrecht-ibm
Copy link
Contributor

@jonathan-albrecht-ibm Can this now be closed in favour of your work in #8962 ?

Yes, #8962 includes these changes so this can be closed.

I have one question about your PR (#8962)... Why the arch is set to z196 as opposed to z10, I have to admit that I am not clear about the numbering of the Z architecture. If I recall, I chose z10 as it was a reasonably old one that was also supported on Travis CI - so I figured it would give us the largest possible backwards/forwards compatibility.

Yes, its definitely a judgment call and I didn't mean to jump the gun on this. z196 is the next generation after z10 and z196 has been around since 2010. I had a look at what the distros support:

Also, I know at least golang's s390x binary dist is built for minimum z196. So my feeling is that z196 is a good default because its not likely you're going to run into a system that supports earlier than that.

@adamretter
Copy link
Collaborator Author

Thanks @jonathan-albrecht-ibm for the explanation, let's leave it as z196 for now.

@adamretter adamretter closed this Nov 8, 2021
yoori pushed a commit to yoori/rocksdb that referenced this pull request Nov 26, 2023
@jonathan-albrecht-ibm
Add support for building on s390x platform (#8962)
cab7cdf 
Summary:
This PR adds support for building on s390x including updating travis CI. It uses the previous work in facebook/rocksdb#6168 and adds some more changes to get all current tests (make check and jni tests) to pass. The tests were run with snappy, lz4, bzip2 and zstd all compiled in.

There are a few pieces still needed to get the travis build working that I don't think I can do. adamretter is this something you could help with?

1. A prebuilt https://rocksdb-deps.s3-us-west-2.amazonaws.com/cmake/cmake-3.14.5-Linux-s390x.deb package
2. A https://hub.docker.com/r/evolvedbinary/rocksjava s390x image

Not sure if there is more required for travis. Happy to help in any way I can.

Pull Request resolved: facebook/rocksdb#8962

Reviewed By: mrambacher

Differential Revision: D31802198

Pulled By: pdillinger

fbshipit-source-id: 683511466fa6b505f85ba5a9964a268c6151f0c2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pdillinger pdillinger pdillinger left review comments

Assignees
No one assigned
Labels
CLA Signed enhancement s390x
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@adamretter @sangitanalkar @edelsohn @jonathan-albrecht-ibm @pdillinger @facebook-github-bot