-
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
[hotfix] fix log file create failed caused by name too long #66
base: FRocksDB-6.20.3
Are you sure you want to change the base?
[hotfix] fix log file create failed caused by name too long #66
Conversation
Hi @Myasuka , can you help me review 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 update, please take a look at my comments.
BTW, it seems many tests not running, @fredia did you ever enable to run CI tests on circle?
94820b9
to
8d3025c
Compare
Hi @Myasuka, I have not modified the configuration of circleCI. It seems that 32(23+9) tests have been executed, and the newst RocksDB have 39 tests, which test do you mean is not executed? |
@fredia Thanks for the clarification, I thought you disable some tests to save the billing on circle before. I think this code change is trivial, @zoltar9264 could you check why so many tests failed after refactoring. |
8d3025c
to
bcec2eb
Compare
Sorry for the late reply @Myasuka , I check the change and find the problem: size_t is unsigned, and "path.length() - (file_name_length_limit - sizeof(suffix))" will never be negative. I added type cast. |
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.
I think some broken tests are due to the changed gcc version. Same code would throw warnings with the latest GCC.
gcc:latest
in https://github.com/ververica/frocksdb/blob/FRocksDB-6.20.3/.circleci/config.yml#L324 changed from gcc-12
to gcc-13
comparing to last successful CI.
Thanks to @Myasuka for helping to locate this problem. If it is true, should we use a specific gcc version, or should we fix the warnings reported in the higher version of gcc? |
From my point of view, I think we can fix the gcc docker image to a fixed version to avoid unnecessary code changes. |
Fix log file create failed caused by name too long by keeping only the last 255 characters , discuss in FLINK-31743 .