Skip to content
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

Refine error handling/cancel in TiFlash MPP system #5095

Closed
windtalker opened this issue Jun 9, 2022 · 2 comments · Fixed by #5286
Closed

Refine error handling/cancel in TiFlash MPP system #5095

windtalker opened this issue Jun 9, 2022 · 2 comments · Fixed by #5286
Assignees
@windtalker
Labels
component/compute type/enhancement The issue or PR belongs to an enhancement.

Comments

@windtalker
Copy link
Contributor

windtalker commented Jun 9, 2022 •
edited
Loading

Enhancement

Currently, error handling/cancel in TiFlash is error prone, it has caused many issues such as #4441, #4219, #4202 etc.
We want to refine the error handling/cancel logical in TiFlash MPP system to make it less error prone.

Some basic ideas:

  • Refine MPPTunnel
    • MPPTunnel has 3 mode: local, sync and async, currently, the implementation of MPPTunnel is based on is_local and is_async flag, which makes the code complex and error prone.
  • MPPTunnel/BlockIO/ExchangeReceiver should be treated as the top level components in MPPTask
  • Each top level components in MPPTask should implement its own cancel and handleError method
  • Like MPPTask::cancel, there should be a method like MPPTask::handleError method to handle errors based on task status
  • Like cancel, there should be a query level error handling method, so once a MPPTask meet error, all the related tasks in the same TiFlash node can see the error and stop running
  • Local tunnel should not introduce direct dependency from send task to receive task
  • As Do not print write to tunnel which is already closed log if query is canceled expectedly before #4208 avoid to print too much meaningless log when error/cancel happens
@windtalker windtalker added the type/enhancement The issue or PR belongs to an enhancement. label Jun 9, 2022
@windtalker windtalker self-assigned this Jun 9, 2022
@windtalker windtalker mentioned this issue Jun 10, 2022
Merged
12 tasks
ti-chi-bot pushed a commit that referenced this issue Jun 10, 2022
@windtalker
move tunnel_map to MPPTunnelSet (#5123)
8916afe 
ref #5095
@windtalker windtalker mentioned this issue Jun 13, 2022
Merged
12 tasks
ti-chi-bot pushed a commit that referenced this issue Jun 14, 2022
@windtalker
Some refinements of mpp_exchange_receiver_map and MPPTunnelSet (#…
864cfe9 
…5132)

ref #5095
@windtalker windtalker mentioned this issue Jun 21, 2022
Merged
12 tasks
ti-chi-bot pushed a commit that referenced this issue Jun 22, 2022
@windtalker
Add MPPReceiverSet, which includes ExchangeReceiver and CoprocessorRe…
e14c677 
…ader (#5175)

ref #5095
@windtalker windtalker mentioned this issue Jun 23, 2022
Merged
12 tasks
ti-chi-bot pushed a commit that referenced this issue Jun 29, 2022
@windtalker
add MPPTask::handleError() (#5202)
7a20339 
ref #5095
@yibin87 yibin87 mentioned this issue Jul 5, 2022
Merged
12 tasks
@windtalker windtalker mentioned this issue Jul 6, 2022
Merged
12 tasks
ti-chi-bot pushed a commit that referenced this issue Jul 7, 2022
@windtalker
refine error message in mpptask (#5304)
3e8df4b 
ref #5095
Lloyd-Pottiger pushed a commit to Lloyd-Pottiger/tiflash that referenced this issue Jul 12, 2022
@hongyunyan @Lloyd-Pottiger
Enhancement: Add how to run integration tests and microbenchmark test…
4e4f3b7 
…s in README (pingcap#5182)

close pingcap#5172, ref pingcap#5178

Enhancement: add a integrated test on DDL module (pingcap#5130)

ref pingcap#5129

Revert "Revise default background threads size" (pingcap#5176)

close pingcap#5177

chore: remove extra dyn cast (pingcap#5186)

close pingcap#5185

Add MPPReceiverSet, which includes ExchangeReceiver and CoprocessorReader (pingcap#5175)

ref pingcap#5095

DDL: Use Column Name Instead of Offset to Find the common handle cluster index (pingcap#5166)

close pingcap#5154

Add random failpoint in critical paths (pingcap#4876)

close pingcap#4807

Segment test framework (pingcap#5150)

close pingcap#5151

optimize ps v3 restore (pingcap#5163)

ref pingcap#4914

Fix build failed (pingcap#5196)

close pingcap#5195

feat: delta tree dispatching (pingcap#5199)

close pingcap#5200

feat: introduce specialized API to write fixed length data rapidly (pingcap#5181)

close pingcap#5183

Add gtest for Limit, TopN, Projection (pingcap#5187) (pingcap#5188)

close pingcap#5187

add `MPPTask::handleError()` (pingcap#5202)

ref pingcap#5095

Check result of starting grpc server (pingcap#5257)

close pingcap#5255

feat: add optimized routines for aarch64 (pingcap#5231)

close pingcap#5240

fix: aarch64-quick-fix (pingcap#5259)

close pingcap#5260

Update client-c to support ipv6 (pingcap#5270)

close pingcap#5247

upgrade prometheus-cpp to v1.0.1 (pingcap#5279)

ref pingcap#2103, close pingcap#5278

Fix README type error (pingcap#5273)

ref pingcap#5178

fix(cmake): make sure libc++ is utilized by tiflash-proxy (pingcap#5281)

close pingcap#5282

fix the wrong order of execution summary for list based executors (pingcap#5242)

close pingcap#5241

Schema: allow loading empty schema diff when the version grows up. (pingcap#5245)

close pingcap#5244

Optimize apply speed under heavy write pressure (pingcap#4883)

ref pingcap#4728

update proxy to raftstore-proxy-6.2 (pingcap#5287)

ref pingcap#4982

Flush segment cache when doing the compaction (pingcap#5284)

close pingcap#5179

metrics: Fix incorrect metrics for delta_merge tasks (pingcap#5061)

close pingcap#5055

dep: upgrade jemalloc (pingcap#5197)

close pingcap#5258

*: TiFlash pagectl/dttool use only-decryption mode (pingcap#5271)

close pingcap#5122

suppresion false positive report from tsan (pingcap#5303)

close pingcap#5088

Refine test framework code and tests (pingcap#5261)

close pingcap#5262

feat: add logical cpu cores and memory into grafana (pingcap#5124)

close pingcap#3821

Implement TimeToSec function push down (pingcap#5235)

close pingcap#5116

feat: implement shiftRight function push down (pingcap#5156)

close pingcap#5100

schema : make update to partition tables when 'set tiflash replica' (pingcap#5267)

close pingcap#5266

Replace initializer_list with vector for planner test framework (pingcap#5307)

close pingcap#5295

KVStore: decouple flush region and CompactLog with a new FFI fn_try_flush_data (pingcap#5283)

ref pingcap#5170

refine error message in mpptask (pingcap#5304)

ref pingcap#5095

Implement ReverseUTF8/Reverse function push down (pingcap#5233)

close pingcap#5111

Optimize comparision for collation `UTF8_BIN` and `UTF8MB4_BIN` (pingcap#5299)

ref pingcap#5294

feat : support set tiflash mode ddl action (pingcap#5256)

ref pingcap#5252

Add non-blocking functions for MPMCQueue (pingcap#5311)

close pingcap#5310

add random segment test for CI weekly (pingcap#5300)

close pingcap#5301

*: tidy FunctionString.cpp (pingcap#5312)

close pingcap#5313

ci: fix check-license github action (pingcap#5318)

close pingcap#5317

update proxy to raftstore-proxy-6.2 (pingcap#5316)

ref pingcap#4982

Change one `additional_input_at_end` to many streams in `ParallelInputsProcessor`  (pingcap#5274)

close pingcap#4856, close pingcap#5263

support fine grained shuffle for window function (pingcap#5048)

close pingcap#5142

feat: pushdown get_format into TiFlash (pingcap#5269)

close pingcap#5115

fix: format throw data truncated error (pingcap#5272)

close pingcap#4891

Print content of columns for gtest (pingcap#5243)

close pingcap#5203

*: also enable O3 for aarch64 (pingcap#5338)

close pingcap#5342

Add debug image build target for CentOS7 (pingcap#5344)

close pingcap#5343

*: mini refactor (pingcap#5326)

close pingcap#4739

Refactor initialize of background pool (pingcap#5190)

close pingcap#5189

delete copy/move ctor of MPMCQueue explicitly (pingcap#5328)

close pingcap#5329

Introduce proxy_server and new-mock-engine-store (pingcap#5319)

ref pingcap#5170

fix: incorrect uptime in grafana panel

Signed-off-by: Lloyd-Pottiger <[email protected]>
This was referenced Jul 13, 2022
Merged
Merged
ti-chi-bot pushed a commit to pingcap/tidb that referenced this issue Jul 13, 2022
@windtalker
executor: parallel cancel mpp query (#36161)
81cf12e 
ref pingcap/tiflash#5095, close #36164
ti-chi-bot pushed a commit that referenced this issue Jul 14, 2022
@yibin87
Refact MPPTunnel class to encapsulate different tunnel mode (#5286)
f65240e 
close #5095
@windtalker
Copy link
Contributor Author

reopen since there are still some work todo.

@windtalker windtalker reopened this Jul 14, 2022
ti-chi-bot pushed a commit that referenced this issue Jul 14, 2022
@windtalker
Refine cancelMPPQuery (#5361)
f97567a 
ref #5095
@windtalker windtalker mentioned this issue Jul 15, 2022
Merged
12 tasks
Lloyd-Pottiger pushed a commit to Lloyd-Pottiger/tiflash that referenced this issue Jul 19, 2022
@windtalker @Lloyd-Pottiger
Add MPPReceiverSet, which includes ExchangeReceiver and CoprocessorRe…
3378905 
…ader (pingcap#5175)

ref pingcap#5095
Lloyd-Pottiger pushed a commit to Lloyd-Pottiger/tiflash that referenced this issue Jul 19, 2022
@windtalker @Lloyd-Pottiger
add MPPTask::handleError() (pingcap#5202)
b4e3163 
ref pingcap#5095
Lloyd-Pottiger pushed a commit to Lloyd-Pottiger/tiflash that referenced this issue Jul 19, 2022
@windtalker @Lloyd-Pottiger
refine error message in mpptask (pingcap#5304)
6d763c1 
ref pingcap#5095
Lloyd-Pottiger pushed a commit to Lloyd-Pottiger/tiflash that referenced this issue Jul 19, 2022
@yibin87 @Lloyd-Pottiger
Refact MPPTunnel class to encapsulate different tunnel mode (pingcap#…
69ae1ca 
…5286)

close pingcap#5095
Lloyd-Pottiger pushed a commit to Lloyd-Pottiger/tiflash that referenced this issue Jul 19, 2022
@windtalker @Lloyd-Pottiger
Refine cancelMPPQuery (pingcap#5361)
0062aad 
ref pingcap#5095
@windtalker windtalker mentioned this issue Aug 17, 2022
Merged
12 tasks
ti-chi-bot pushed a commit that referenced this issue Aug 17, 2022
@windtalker
Fix bug that mpp_task is not move to cancel thread due to llvm SOO (#…
0933d34 
…5637)

ref #5095, close #5638
@ti-chi-bot ti-chi-bot mentioned this issue Aug 17, 2022
Closed
12 tasks
@windtalker windtalker mentioned this issue Aug 18, 2022
Merged
12 tasks
ti-chi-bot pushed a commit that referenced this issue Aug 18, 2022
@windtalker
Remove MPPTaskManager::findTaskWithTimeout (#5647)
b6536f6 
ref #5095
@windtalker windtalker mentioned this issue Aug 18, 2022
Merged
12 tasks
ti-chi-bot pushed a commit that referenced this issue Aug 18, 2022
@windtalker
Fix data race in tunnel_set/receiver_set and establish call data (#5650)
7428d37 
ref #5095, close #5651
@ti-chi-bot ti-chi-bot mentioned this issue Aug 18, 2022
Closed
12 tasks
ti-chi-bot pushed a commit that referenced this issue Aug 23, 2022
@windtalker
refine error handling for MPP query (#5374)
8e92ffb 
ref #5095
@windtalker windtalker mentioned this issue Aug 26, 2022
Merged
12 tasks
ti-chi-bot pushed a commit that referenced this issue Aug 26, 2022
@windtalker
fix unstable mpp fail tests (#5709)
61e436b 
ref #5095, close #5708
@hehechen hehechen mentioned this issue Aug 30, 2022
Closed
@windtalker windtalker mentioned this issue Sep 5, 2022
Merged
12 tasks
ti-chi-bot pushed a commit that referenced this issue Sep 5, 2022
@windtalker
fix unstable tests (#5791)
4ee341d 
ref #5095, ref #5708
@windtalker windtalker mentioned this issue Sep 8, 2022
Merged
12 tasks
ti-chi-bot pushed a commit that referenced this issue Sep 8, 2022
@windtalker
Refine error propagation in local tunnel (#5824)
462b545 
ref #5095
@windtalker windtalker mentioned this issue Sep 20, 2022
Merged
12 tasks
ti-chi-bot pushed a commit that referenced this issue Sep 20, 2022
@windtalker
add mpp query count metrics (#5959)
1223584 
ref #5095
@windtalker windtalker mentioned this issue Sep 20, 2022
Merged
12 tasks
ti-chi-bot pushed a commit that referenced this issue Sep 22, 2022
@windtalker
Do not use extra threads when cancel mpp query (#5966)
988cde9 
ref #5095, close #5631
This was referenced Sep 22, 2022
Closed
Closed
@windtalker
Copy link
Contributor Author

close since all the tasks are done

@SeaRise SeaRise mentioned this issue Dec 13, 2022
Merged
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@windtalker windtalker

Labels
component/compute type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Refact MPPTunnel class to encapsulate different tunnel mode yibin87/tiflash
2 participants
@windtalker @SeaRise