Cancel query on Ctrl-C when --pager used by GSokol · Pull Request #88935 · ClickHouse/ClickHouse

GSokol · 2025-10-24T10:45:07Z

Changelog category (leave one):

Improvement

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

User can now cancel the query by pressing Ctrl-C when the pager is running. Resolves #80778

Documentation entry for user-facing changes

Documentation is written (mandatory for new features)

Details

Trying to process SIGINT query and terminate the pager. I have great concerns about what should be done: different pagers behave differently. Some could pass-through the signal, some -- not. Different signals could be handled differently, etc. The patch is more like a fast-and-dirty fix of particular use cases:

pagers: more, less
queries:

SELECT 3
SELECT * FROM numbers(1e12) format Null
SELECT * FROM numbers(300)

behavior:

try to wait for result (for short queries) and exit from pager
for long-running queries, try cancel and then assert that pager is terminated

Resolves #80778

Trying to process SIGINT query and terminate the pager. I have a great concerns about what should be done: different pages behaves differently. Some could path-through the signal, some -- not. Different signals could be handled differently, etc. The patch is more like fast-and-dirty fix of particular use cases: pagers: `more`, `less` queries: - `SELECT 3` - `SELECT * FROM numbers(1e12) format Null` - `SELECT * FROM numbers(300)` behavior: - try to wait for result (for short queries) and exit from pager - for long-running queries, try cancel and then assert that pager is terminated

GSokol · 2025-10-24T10:47:07Z

Relates comment: https://github.com/ClickHouse/ClickHouse/pull/55193/files#r1361223112

GSokol · 2025-10-24T13:08:29Z

@azat, could you take a look?

azat

Thanks for taking a look!

Looks OK, but let's add a test, I do not want to break it again (you can take a look at expect tests, and use query like select sleepEachRow(1) from numbers(30) settings max_block_size=1, function_sleep_max_microseconds_per_block=30e6)

Can you also address minor issues here:

missing redraw in case of cancellation, though I cannot reproduce it, but after a simple query (select sleepEachRow(1) from numbers(3) settings max_block_size=1) there was not prompt
and sometimes there are extra new lines - https://pastila.nl/?07b9abe6/d5d459bed20554da7ced8d4559429226#mU1u2N+22zv57/tl9tLobw==

GSokol · 2025-10-26T20:38:52Z

@azat, here is the test. BTW, I struggled to test proper less exit q, no clue, how to do it. Tried send -- "q" and echo 'q' >> /proc/${LESS_PID}/fd/0.

tests/queries/0_stateless/03701_pager_proper_termination.expect

azat · 2025-10-27T11:00:54Z

I struggled to test proper less exit q, no clue, how to do it. Tried send -- "q" and echo 'q' >> /proc/${LESS_PID}/fd/0.

Hm, I think it should simply by sending q to the cilent, but the problem is that you need to make sure that the pager already started by the client, anyway, it is not a requirement for this test

azat · 2025-10-27T11:01:18Z

@GSokol what about two other issues?

clickhouse-gh · 2025-10-27T11:01:49Z

Workflow [PR], commit [8099349]

Summary: ❌

job_name	test_name	status	info
Stateless tests (amd_binary, old analyzer, s3 storage, DatabaseReplicated, parallel)		failure
	01201_read_single_thread_in_order	FAIL	cidb, flaky
Integration tests (amd_asan, old analyzer, 2/6)		failure
	test_storage_kafka/test_batch_slow_6.py::test_kafka_rebalance[generate_new_create_table_query-{}.*Saved offset]	FAIL	cidb
Integration tests (amd_asan, old analyzer, 3/6)		failure
	test_ytsaurus/test_dictionaries.py::test_yt_dictionary_id[False-FLAT]	FAIL	cidb
	test_ytsaurus/test_dictionaries.py::test_yt_dictionary_id[True-FLAT]	FAIL	cidb
	test_ytsaurus/test_dictionaries.py::test_yt_dictionary_id[False-HASHED]	FAIL	cidb
	test_ytsaurus/test_dictionaries.py::test_yt_dictionary_id[True-HASHED]	FAIL	cidb
	test_ytsaurus/test_dictionaries.py::test_yt_dictionary_id[False-HASHED_ARRAY]	FAIL	cidb
	test_ytsaurus/test_dictionaries.py::test_yt_dictionary_id[True-HASHED_ARRAY]	FAIL	cidb
	test_ytsaurus/test_dictionaries.py::test_yt_dictionary_complex_key[True-COMPLEX_KEY_HASHED]	FAIL	cidb
	test_ytsaurus/test_dictionaries.py::test_yt_dictionary_complex_key[False-COMPLEX_KEY_HASHED]	FAIL	cidb
	test_ytsaurus/test_dictionaries.py::test_yt_dictionary_complex_key[True-COMPLEX_KEY_HASHED_ARRAY]	FAIL	cidb
	test_ytsaurus/test_dictionaries.py::test_yt_dictionary_complex_key[False-COMPLEX_KEY_HASHED_ARRAY]	FAIL	cidb
	8 more test cases not shown
Integration tests (amd_asan, old analyzer, 4/6)		failure
	test_scheduler_cpu_preemptive/test.py::test_downscaling[cpu-slot-preemption-timeout-1ms]	FAIL	cidb, flaky
	test_backup_restore_on_cluster/test_slow_rmt.py::test_replicated_database_async	FAIL	cidb
	OOM in dmesg	FAIL	cidb
Integration tests (arm_binary, distributed plan, 3/4)		failure
	test_backup_restore_on_cluster/test.py::test_async_backups_to_same_destination[http-False]	FAIL	cidb, flaky

GSokol · 2025-10-27T11:13:55Z

@GSokol what about two other issues?

So:

missing redraw in case of cancellation, though I cannot reproduce it, but after a simple query (select sleepEachRow(1) from numbers(3) settings max_block_size=1) there was not prompt

As I understand this expectation insures, that the prompt is alweys remndered, isn't it?

and sometimes there are extra new lines - https://pastila.nl/?07b9abe6/d5d459bed20554da7ced8d4559429226#mU1u2N+22zv57/tl9tLobw==

I would check that there is no new line in the end.

azat · 2025-10-27T11:31:28Z

As I understand this expectation insures, that the prompt is alweys remndered, isn't it?

Not exactly, when you launch client it does not execute pager yet, while the problem I found was that after pager had been executed and I pressed Ctrl-C, there was no prompt. Maybe you can try to reproduce it.

azat · 2025-10-27T11:32:59Z

Also about changelog category, we are using Bug fix category only for major bugs, let's use Improvement for this one (and don't forget to set changelog entry, otherwise CI will not start)

GSokol · 2025-10-27T12:20:55Z

Not exactly, when you launch client it does not execute pager yet, while the problem I found was that after pager had been executed and I pressed Ctrl-C, there was no prompt. Maybe you can try to reproduce it.

Sorry, the wrong line number. Here is the proper one.

tests/queries/0_stateless/03701_pager_proper_termination.expect

azat · 2025-10-29T13:38:35Z

try to wait for result (for short queries) and exit from pager

@GSokol can you point me to a relevant code in the patch for this behavior?

…ination

azat · 2025-10-31T08:09:47Z

@GSokol I've fixed the test and now it is ready to land! Thanks for taking a look into this.

azat self-assigned this Oct 25, 2025

azat reviewed Oct 25, 2025

View reviewed changes

Add tests

91856d7

azat reviewed Oct 27, 2025

View reviewed changes

tests/queries/0_stateless/03701_pager_proper_termination.expect Outdated Show resolved Hide resolved

tests/queries/0_stateless/03701_pager_proper_termination.expect Outdated Show resolved Hide resolved

azat added the can be tested Allows running workflows for external contributors label Oct 27, 2025

Test improvement

7c05d5c

clickhouse-gh bot added the pr-improvement Pull request with some product improvements label Oct 27, 2025

azat changed the title ~~Cancel query on Ctr-C when --pager used~~ Cancel query on Ctrl-C when --pager used Oct 29, 2025

azat added 2 commits October 29, 2025 14:35

Fix 03701_pager_proper_termination test

7fd2607

Add comments for conditional pager waiting

61a7362

azat reviewed Oct 29, 2025

View reviewed changes

tests/queries/0_stateless/03701_pager_proper_termination.expect Outdated Show resolved Hide resolved

Rellocate headers in ClientBase

383cbd2

azat approved these changes Oct 29, 2025

View reviewed changes

Add less to CI images

13ae058

clickhouse-gh bot added the manual approve Manual approve required to run CI label Oct 29, 2025

azat added 2 commits October 29, 2025 18:27

tests: attempt to mitigate "WARNING: terminal is not fully functional"

82d047d

tests: ensure that less will be terminated in 03701_pager_proper_term…

a7f81a8

…ination

azat enabled auto-merge October 30, 2025 12:51

tests: even more reliable 03701_pager_proper_termination

8099349

azat added this pull request to the merge queue Oct 31, 2025

Merged via the queue into ClickHouse:master with commit 365fc6d Oct 31, 2025
118 of 124 checks passed

robot-clickhouse-ci-1 added the pr-synced-to-cloud The PR is synced to the cloud repo label Oct 31, 2025

Conversation

GSokol commented Oct 24, 2025 • edited by azat Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelog category (leave one):

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Documentation entry for user-facing changes

Details

Uh oh!

GSokol commented Oct 24, 2025

Uh oh!

GSokol commented Oct 24, 2025

Uh oh!

azat left a comment

Choose a reason for hiding this comment

Uh oh!

GSokol commented Oct 26, 2025

Uh oh!

Uh oh!

Uh oh!

azat commented Oct 27, 2025

Uh oh!

azat commented Oct 27, 2025

Uh oh!

clickhouse-gh bot commented Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GSokol commented Oct 27, 2025

Uh oh!

azat commented Oct 27, 2025

Uh oh!

azat commented Oct 27, 2025

Uh oh!

GSokol commented Oct 27, 2025

Uh oh!

Uh oh!

azat commented Oct 29, 2025

Uh oh!

azat commented Oct 31, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

GSokol commented Oct 24, 2025 •

edited by azat

Loading

clickhouse-gh bot commented Oct 27, 2025 •

edited

Loading