Try to squash matview concurrent inserts

[antonio2368]

Changelog category (leave one):

• Improvement

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

Squash data from all threads before inserting to materialized views depending on the settings min_insert_block_size_rows_for_materialized_views and min_insert_block_size_bytes_for_materialized_views.
Previously, if parallel_view_processing was enabled, each thread inserting to a specific materailized view would squash insert independently which could lead to higher number of generated parts.

Documentation entry for user-facing changes

• Documentation is written (mandatory for new features)

[clickhouse-gh]

Workflow [PR], commit [7deeb3a]

Summary: ❌

job_name	test_name	status	info	comment
Upgrade check (amd_msan)		failure
	Killed by signal (in clickhouse-server.log)	FAIL
	Fatal message in clickhouse-server.log (see fatal_messages.txt)	FAIL
	Killed by signal (output files)	FAIL
	Found signal in gdb.log	FAIL

[den-crane]

How it will work if there is multiple inserts and some of the inserts have transactions and able to initiate rollback after insertion?

[antonio2368]

Sorry for not being clear, I'll write proper changelog once the PR is ready.
This squashes insert data from same inserts but different threads.
Currently, we create squashing transform per thread so each of it will squash the data independently.
I'm trying to see if I can squash everything in one place to reduce number of generated parts, especially if MV generates much smaller parts than the source insert.

[antonio2368]

@CheSema do you think we need a setting to have old behavior?
Even though this is better when it comes to number of parts it could increase insert latency depending on the SELECT queries done by the MVs.

[CheSema]

Sema Checherinda do you think we need a setting to have old behavior? Even though this is better when it comes to number of parts it could increase insert latency depending on the SELECT queries done by the MVs.

I'm really worried here about deduplication. I know that the order of the resulting chunks in inner query has to be preserved and thy can not be mixed. So that feature could be incompatible with the way how we deduplicate inserted data.

[CheSema]

what if we need more inserting threads here?

[antonio2368]

I'm really worried here about deduplication. I know that the order of the resulting chunks in inner query has to be preserved and thy can not be mixed. So that feature could be incompatible with the way how we deduplicate inserted data.

It will be disabled if we deduplication is enabled. I'm thinking about adding setting that could disable squashing in such way even with disabled deduplication.

[CheSema]

I see, you do it with respect to deduplication.

[CheSema]

this is not clear why do we need this changes?
why do we have a result on Finish stage?

[antonio2368]

PlanSquashingTransform was IInflatingTransform because we don't create chunk until we have enough data. To properly handle errors from MV it needed to become ExceptionKeepingTransform.
As the logic of IInflatingTransform is much simple, the easiest thing for me was to add the logic from it to ExceptionKeepingTransform.

[CheSema]

As I understand the code the setting insert thread count was introduced not for speeding up inner queries, it does it as side effect, but mainly as making writing to destination table concurrently.

Many be we need here more detailed settings like:
inner_mv_queries_concurency
insertion_concurency

[antonio2368]

As I understand the code the setting insert thread count was introduced not for speeding up inner queries, it does it as side effect, but mainly as making writing to destination table concurrently.

Yes, but if you have chained MVs, with this PR it will squash data from different threads which means instead of running 4 smaller SELECTs (and creating more parts) you run 1 heavy SELECT (and creating only 1 part).
So you end up doing more during the insert instead of during merges and SELECTs on the destination table.
It general this should be fine, creating fewer parts is always a win but it could confuse some users.

And I agree for the settings, it's a bit confusing how max_insert_threads interacts with MVs. Let's do it in different PR not to clutter this one.