Skip to content

Fix memory leak in registerDiskS3#11074

Merged
alexey-milovidov merged 2 commits intoClickHouse:masterfrom
Jokser:memory-leak-in-register-disk-s3
May 21, 2020
Merged

Fix memory leak in registerDiskS3#11074
alexey-milovidov merged 2 commits intoClickHouse:masterfrom
Jokser:memory-leak-in-register-disk-s3

Conversation

@Jokser
Copy link
Copy Markdown
Contributor

@Jokser Jokser commented May 20, 2020

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

Changelog category (leave one):

  • Bug Fix

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fixed memory leak in registerDiskS3

Detailed description / Documentation draft:
Memory leak in registerDiskS3 that was found here #11029 (comment) is fixed.
Improved S3 proxy resolver error handling and usability

@blinkov blinkov added the pr-bugfix Pull request with bugfix, not backported by default label May 20, 2020
@azat
Copy link
Copy Markdown
Member

azat commented May 20, 2020

@Jokser Maybe test with unavailable S3 worth adding? (that triggers the initial issue)

@alexey-milovidov alexey-milovidov self-assigned this May 20, 2020
@alexey-milovidov
Copy link
Copy Markdown
Member

+1, let's add a test that can reproduce the leak before this change (no need to validate it, just let's add this test case).

@alesapin
Copy link
Copy Markdown
Member

alesapin commented May 21, 2020

Thie leak issue can be detected with test_replicated_merge_tree_s3/test.py::test_insert_select_replicated (can be found on current master).

@alexey-milovidov alexey-milovidov merged commit 4a237fa into ClickHouse:master May 21, 2020
@azat
Copy link
Copy Markdown
Member

azat commented May 21, 2020

Thie leak issue can be detected with test_replicated_merge_tree_s3/test.py::test_insert_select_replicated (can be found on current master).

@alesapin AFAIU it will trigger the issue only when s3 (minio) will not be available, am I missing something?

For example this build does not have this patch and successfully passes (And the problem that the leak happens only on abnormal shutdown)

nikitamikhaylov pushed a commit to nikitamikhaylov/ClickHouse that referenced this pull request May 23, 2020
…er-disk-s3

Fix memory leak in registerDiskS3

(cherry picked from commit 4a237fa)
nikitamikhaylov added a commit that referenced this pull request May 23, 2020
* Merge pull request #10910 from filimonov/kafka_drop_hang_fix

Fix for the hang during deletion of engine=Kafka

(cherry picked from commit 0fd0711)

* Merge pull request #10986 from ClickHouse/try-fix-use-after-free-mergetree

Try to fix use-after-free error in MergeTree

(cherry picked from commit 073dc2e)

* Merge pull request #11048 from filimonov/kafka_missed_data_during_drop

Fixes the potential missed data during termination of Kafka engine table

(cherry picked from commit b82c633)

* Merge pull request #11109 from ClickHouse/less_verbose_logging

Less verbose logging in mutation finalization task

(cherry picked from commit 1906762)

* Merge pull request #11074 from Jokser/memory-leak-in-register-disk-s3

Fix memory leak in registerDiskS3

(cherry picked from commit 4a237fa)

* Merge pull request #11038 from Enmk/parseDateTime64BestEffort_fix

Fixed parseDateTime64BestEffort implementation

(cherry picked from commit 5a0f356)

Co-authored-by: alexey-milovidov <[email protected]>
Co-authored-by: alesapin <[email protected]>
Co-authored-by: Vitaly Baranov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-docs-needed pr-bugfix Pull request with bugfix, not backported by default

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants