Skip to content

Ignore parse error in system.distributed_ddl_queue #79369

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.

Sign up for GitHub

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

Jump to bottom
Merged
azat merged 15 commits into from
Jun 1, 2025
Merged

Ignore parse error in system.distributed_ddl_queue #79369

azat merged 15 commits into from
Jun 1, 2025

Conversation

@ilejn
Copy link
Contributor

@ilejn ilejn commented Apr 21, 2025 •
edited by azat
Loading

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

Fixes: #75011

@ilejn
Copy link
Contributor Author

ilejn commented Apr 21, 2025

Addresses #75011

@azat azat self-assigned this Apr 21, 2025
azat
azat reviewed Apr 21, 2025
View reviewed changes
@ilejn
Copy link
Contributor Author

ilejn commented Apr 28, 2025

Hello @azat ,
further suggestions?

azat
azat reviewed May 25, 2025
View reviewed changes
Copy link
Member

@azat azat left a comment

Choose a reason for hiding this comment

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

Sorry for the delay (was on vacation)
LGTM, but let's also make test more stable

@azat azat added the can be tested Allows running workflows for external contributors label May 26, 2025
@clickhouse-gh
Copy link

clickhouse-gh bot commented May 26, 2025 •
edited
Loading

Workflow [PR], commit [e6180de]

@clickhouse-gh clickhouse-gh bot added the pr-not-for-changelog This PR should not be mentioned in the changelog label May 26, 2025
@azat
Copy link
Member

azat commented May 28, 2025

Integration tests failures is related, @ilejn can you please take a look?

@ilejn
Copy link
Contributor Author

ilejn commented May 28, 2025

Please, advise if I should trigger the build.
I don't think that any test failure is related, but 'server died' looks too bad.

azat
azat reviewed May 29, 2025
View reviewed changes
@ilejn
Copy link
Contributor Author

ilejn commented May 30, 2025

AST fuzzer thing is not related.

@azat azat added this pull request to the merge queue May 30, 2025
@azat azat removed this pull request from the merge queue due to a manual request May 30, 2025
azat
azat reviewed May 30, 2025
View reviewed changes
@ilejn
Copy link
Contributor Author

ilejn commented May 31, 2025

@azat , imagine that CH parses queries in system.query_log and in case of an exception returns error to client.
This is exactly what is happening here.

@azat azat enabled auto-merge June 1, 2025 06:41
@azat azat added this pull request to the merge queue Jun 1, 2025
@azat
Copy link
Member

azat commented Jun 1, 2025

imagine that CH parses queries in system.query_log and in case of an exception returns error to client.
This is exactly what is happening here.

I understand your point, but I still believe that until we have some solid examples, we should not ignore other errors.

I am concerned that someone might rely on the cluster field and would prefer to know whether the failure is temporary (and the query can be retried) or permanent.

Merged via the queue into ClickHouse:master with commit ce31d75 Jun 1, 2025
120 checks passed
@robot-ch-test-poll1 robot-ch-test-poll1 added the pr-synced-to-cloud The PR is synced to the cloud repo label Jun 1, 2025
ilejn pushed a commit to Altinity/ClickHouse that referenced this pull request Jun 5, 2025
@azat @ilejn
Merge pull request ClickHouse#79369 from ilejn/ignore_error_distribut…
7a49bd3 
…ed_ddl_queue

Ignore parse error in system.distributed_ddl_queue

conflict resolved
zvonand added a commit to Altinity/ClickHouse that referenced this pull request Jun 19, 2025
@zvonand
Merge pull request #843 from Altinity/backports/24.8.14/79369
3d79cdb 
24.8.14 Backport of ClickHouse#79369 -- Ignore parse error in system.distributed_ddl_queue
ilejn pushed a commit to Altinity/ClickHouse that referenced this pull request Jul 9, 2025
@azat @ilejn
Merge pull request ClickHouse#79369 from ilejn/ignore_error_distribut…
026cc82 
…ed_ddl_queue

Ignore parse error in system.distributed_ddl_queue
zvonand added a commit to Altinity/ClickHouse that referenced this pull request Jul 14, 2025
@zvonand
Merge pull request #889 from Altinity/backports/25.3.5/79369
84c3a76 
25.3.5 Backport of ClickHouse#79369 -- Ignore parse error in system.distributed_ddl_queue
zvonand pushed a commit to Altinity/ClickHouse that referenced this pull request Jul 16, 2025
@azat @zvonand
Merge pull request ClickHouse#79369 from ilejn/ignore_error_distribut…
00773a6 
…ed_ddl_queue

Ignore parse error in system.distributed_ddl_queue
zvonand added a commit to Altinity/ClickHouse that referenced this pull request Jul 17, 2025
@zvonand
Merge pull request #914 from Altinity/backports/25.3.6/79369
f48ebac 
25.3.6 Backport of ClickHouse#79369 -- Ignore parse error in system.distributed_ddl_queue
ilejn pushed a commit to Altinity/ClickHouse that referenced this pull request Nov 7, 2025
@azat @ilejn
Merge pull request ClickHouse#79369 from ilejn/ignore_error_distribut…
0492f67 
…ed_ddl_queue

Ignore parse error in system.distributed_ddl_queue
ilejn added a commit to Altinity/ClickHouse that referenced this pull request Nov 7, 2025
@ilejn
Revert "Merge pull request ClickHouse#79369 from ilejn/ignore_error_d…
2df8357 
…istributed_ddl_queue"

This reverts commit 0492f67.
ilejn pushed a commit to Altinity/ClickHouse that referenced this pull request Nov 7, 2025
@azat @ilejn
Merge pull request ClickHouse#79369 from ilejn/ignore_error_distribut…
5238553 
…ed_ddl_queue

Ignore parse error in system.distributed_ddl_queue
zvonand added a commit to Altinity/ClickHouse that referenced this pull request Nov 10, 2025
@zvonand
Merge pull request #1128 from Altinity/backports/25.3.8/79369_system_…
ca7fab7 
…distributed_ddl_queue

25.3.8 Backport of ClickHouse#79369 -- Ignore parse error in system.distributed_ddl_queue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@azat azat azat approved these changes

Assignees

@azat azat

Labels

can be tested Allows running workflows for external contributors pr-not-for-changelog This PR should not be mentioned in the changelog pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

system.distributed_ddl_queue parses the queries that it finds in ZK

3 participants

@ilejn @azat @robot-ch-test-poll1