Skip to content

The behavior of clickhouse-keeper to create znode is consistent with zookeeper#27125

Merged
alesapin merged 6 commits intoClickHouse:masterfrom
nicelulu:fix_create_znode
Aug 19, 2021
Merged

The behavior of clickhouse-keeper to create znode is consistent with zookeeper#27125
alesapin merged 6 commits intoClickHouse:masterfrom
nicelulu:fix_create_znode

Conversation

@nicelulu
Copy link
Copy Markdown
Contributor

@nicelulu nicelulu commented Aug 3, 2021 •
edited by alexey-milovidov
Loading

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:
Improved the existence condition judgment and empty string node judgment when clickhouse-keeper creates znode.

@robot-clickhouse robot-clickhouse added the pr-improvement Pull request with some product improvements label Aug 3, 2021
@nicelulu
Copy link
Copy Markdown
Contributor Author

nicelulu commented Aug 3, 2021 •
edited
Loading 

2021.08.03 15:25:57.116413 [ 5657 ] {2f0fdf4a-1326-482a-8e84-87c4dca1794c} <Error> executeQuery: Code: 999. Coordination::Exception: Bad arguments, path: /clickhouse/. (KEEPER_EXCEPTION) (version 21.9.1.7648) (from [::1]:48922) (comment: '/usr/share/clickhouse-test/queries/0_stateless/01034_move_partition_from_table_zookeeper.sh') (in query: CREATE TABLE src (p UInt64, k String, d UInt64) ENGINE = ReplicatedMergeTree('/clickhouse//src3', '1') PARTITION BY p ORDER BY (p, k, d);), Stack trace (when copying this message, always include the lines below):


ReplicatedMergeTree('/clickhouse//src3', '1')

Maybe it is creating /clickhouse/, which will also cause the same error under zookeeper: Bad arguments

@nicelulu nicelulu mentioned this pull request Aug 4, 2021
Merged
@nicelulu
Copy link
Copy Markdown
Contributor Author

nicelulu commented Aug 4, 2021

ZooKeeper has the same error.

CREATE TABLE src
(
    `p` UInt64,
    `k` String,
    `d` UInt64
)
ENGINE = ReplicatedMergeTree('/clickhouse//src3', '1')
PARTITION BY p
ORDER BY (p, k, d)

Query id: f2f0eb98-2848-44ae-9255-7c53371b993a


Received exception from server (version 21.3.13):
Code: 999. DB::Exception: Received from 10.10.10.10:9000. DB::Exception: Bad arguments, path: /clickhouse/.

0 rows in set. Elapsed: 0.007 sec.

tavplubix
tavplubix approved these changes Aug 16, 2021
View reviewed changes
tests/integration/test_keeper_back_to_back/test.py Outdated
except Exception as ex:
fake_throw = True

assert genuine_throw == fake_throw
Copy link
Copy Markdown
Member

@tavplubix tavplubix Aug 16, 2021

Choose a reason for hiding this comment

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

It would be better to check that both genuine_throw and fake_throw are True

Copy link
Copy Markdown
Contributor Author

@nicelulu nicelulu Aug 18, 2021

Choose a reason for hiding this comment

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

Thanks.

@tavplubix tavplubix self-assigned this Aug 16, 2021
@alesapin
Copy link
Copy Markdown
Member

alesapin commented Aug 18, 2021

@Mergifyio update

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Aug 18, 2021

Command update: success

Branch already up to date

@alesapin
Copy link
Copy Markdown
Member

alesapin commented Aug 18, 2021

@Mergifyio update

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Aug 18, 2021

Command update: success

Branch has been successfully updated

alesapin
alesapin approved these changes Aug 18, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@alesapin alesapin left a comment

Choose a reason for hiding this comment

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

Nice catch, LGTM

@alesapin
Copy link
Copy Markdown
Member

alesapin commented Aug 19, 2021

Unrelated failures in tests.

@alesapin alesapin merged commit 09df501 into ClickHouse:master Aug 19, 2021
@alexey-milovidov alexey-milovidov mentioned this pull request Nov 24, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alesapin alesapin alesapin approved these changes

@tavplubix tavplubix tavplubix approved these changes

Assignees

@tavplubix tavplubix

Labels

pr-improvement Pull request with some product improvements

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@nicelulu @alesapin @tavplubix @alexey-milovidov @robot-clickhouse