-
Notifications
You must be signed in to change notification settings - Fork 49
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
[FLINK-33811] Fix the broken CI #71
Conversation
2942763
to
aa0dbef
Compare
@Zakelly could you please provide the commits you refer to? |
I update them in description of this PR. |
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.
Thanks for the quick work, please take a look at my comments.
python-version: "3.7.8" | ||
steps: | ||
- pre-steps | ||
# Temporarily disable warning as error since we upgrade the image version, and there are lots of new introduced warnings. |
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.
Could you please leave this change to a separate commit as we should try to fix the warnings in general cases.
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.
Sure. But that's a big TODO since there are lots of warnings scattered everywhere. Those warnings are fixed by upstream rocksdb in 8.x version, so if we migrate to 8.9 or later, I suggest we leave the 6.20.3 branch as it is.
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.
Got it, agreed.
@@ -295,7 +289,7 @@ jobs: | |||
- pre-steps | |||
- install-gflags | |||
- install-clang-10 | |||
- run: COMPILE_WITH_UBSAN=1 OPT="-fsanitize-blacklist=.circleci/ubsan_suppression_list.txt" CC=clang-10 CXX=clang++-10 ROCKSDB_DISABLE_ALIGNED_NEW=1 USE_CLANG=1 make V=1 -j32 ubsan_check | .circleci/cat_ignore_eagain # aligned new doesn't work for reason we haven't figured out |
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.
Since .circleci/cat_ignore_eagain
is useless, can we just delete that file like facebook/rocksdb#9531?
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.
Thanks for the update, LGTM.
@Zakelly Thanks for your work, LGTM. |
Fix FLINK-33811
Picked changes from 6.x and 8.x branchs of rocksdb. Since there are too many diffs between this repo and rocksdb, I only pick the useful lines from it without original commit messages.
Change Log