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

Compressed logs for keeper #29223

Merged
merged 9 commits into from
Sep 24, 2021
Merged

Compressed logs for keeper #29223

merged 9 commits into from
Sep 24, 2021

Conversation

alesapin
Copy link
Member

@alesapin alesapin commented Sep 21, 2021

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Changelog category (leave one):

  • Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Add compress_logs settings for clickhouse-keeper which allow to compress clickhouse-keeper logs in ZSTD . Implements: #26977.

@robot-clickhouse robot-clickhouse added the pr-improvement Pull request with some product improvements label Sep 21, 2021
@alesapin alesapin added the jepsen-test Need to test this PR with jepsen tests label Sep 21, 2021
@nikitamikhaylov nikitamikhaylov self-assigned this Sep 21, 2021
@alesapin
Copy link
Member Author

For some reason zstd still argues, while keeper is able to read compressed logs:

*** zstd command line interface 64-bits v1.3.3, by Yann Collet ***                                                                                                                                                                                                               
log_1_100000.bin.zst : 14 MB...     log_1_100000.bin.zst : Read error (39) : premature end                                                                                                                                                                                       ```
Maybe I have stale version.

@alesapin
Copy link
Member Author

*** zstd command line interface 64-bits v1.4.9, by Yann Collet ***
0001_200000.bin.zstd : 43 MB...     0001_200000.bin.zstd : Read error (39) : premature end 

@alesapin alesapin added the force tests The label does nothing, NOOP, None, nil label Sep 22, 2021
@alesapin
Copy link
Member Author

@Mergifyio update

@mergify
Copy link
Contributor

mergify bot commented Sep 22, 2021

Command update: success

Branch has been successfully updated

@alesapin
Copy link
Member Author

alesapin commented Sep 23, 2021

Ok, logs became much smaller. Now need to make snapshots compressed also with ZSTD (next PR).

@nikitamikhaylov
Copy link
Member

Discussed a bit in private chat. Need to fix some issue.

@filimonov
Copy link
Contributor

BTW - why not LZ4? It's few times faster (quite important for logs)

@alesapin
Copy link
Member Author

alesapin commented Sep 24, 2021

BTW - why not LZ4? It's few times faster (quite important for logs)

Simpler stream compression, good compression ratio (about x10), small amount of data.

@alesapin alesapin merged commit 3614fb9 into master Sep 24, 2021
@alesapin alesapin deleted the compressed_logs branch September 24, 2021 13:03
@alexey-milovidov
Copy link
Member

alexey-milovidov commented Sep 24, 2021

LZ4 is too fast for ZooKeeper logs.
If we have around a gigabyte of ZooKeeper logs per second - it's way more significant problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
force tests The label does nothing, NOOP, None, nil jepsen-test Need to test this PR with jepsen tests pr-improvement Pull request with some product improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants