Skip to content

Fix undefined-behavior in DirectoryMonitor (for exponential back off)#26464

Merged
alexey-milovidov merged 1 commit intoClickHouse:masterfrom
azat:ubsan-dir-mon-fix
Jul 17, 2021
Merged

Fix undefined-behavior in DirectoryMonitor (for exponential back off)#26464
alexey-milovidov merged 1 commit intoClickHouse:masterfrom
azat:ubsan-dir-mon-fix

Conversation

@azat
Copy link
Copy Markdown
Member

@azat azat commented Jul 17, 2021

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fix undefined-behavior in DirectoryMonitor (for exponential back off)

Detailed description / Documentation draft:
UBsan reports 1:

../src/Storages/Distributed/DirectoryMonitor.cpp:435:54: runtime error: 2.30584e+19 is outside the range of representable values of type 'unsigned long'

UBsan reports [1]:

    ../src/Storages/Distributed/DirectoryMonitor.cpp:435:54: runtime error: 2.30584e+19 is outside the range of representable values of type 'unsigned long'

  [1]: https://clickhouse-test-reports.s3.yandex.net/0/10f3500b3be73c9498d994d189784c7d44ed6793/stress_test_(undefined).html#fail1
@robot-clickhouse robot-clickhouse added the pr-not-for-changelog This PR should not be mentioned in the changelog label Jul 17, 2021
Copy link
Copy Markdown
Member

@alexey-milovidov alexey-milovidov left a comment

Choose a reason for hiding this comment

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

Ok.

Alternatively (and more simply) we can cap error_count (wth e.g. 10) before passing to the formula.

@alexey-milovidov alexey-milovidov self-assigned this Jul 17, 2021
@alexey-milovidov alexey-milovidov merged commit b16e015 into ClickHouse:master Jul 17, 2021
@azat azat deleted the ubsan-dir-mon-fix branch July 19, 2021 05:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-not-for-changelog This PR should not be mentioned in the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants