table: Add CachedTableSupport and TemporaryTableSupport for MutateContext
#54900
Conversation
ti-chi-bot
bot
added
release-note-none
|
Hi @lcwangchao. Thanks for your PR. PRs from untrusted users cannot be marked as trusted with I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
lcwangchao
force-pushed
the
mutatectx_temp_cached_table
branch
2 times, most recently
from
fbe6501
to
f1d2853
Compare
lcwangchao
force-pushed
the
mutatectx_temp_cached_table
branch
from
f1d2853
to
f78c4a0
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #54900 +/- ##
=================================================
- Coverage 72.8511% 56.0800% -16.7712%
=================================================
Files 1558 1683 +125
Lines 438406 613413 +175007
=================================================
+ Hits 319384 344002 +24618
- Misses 99322 245909 +146587
- Partials 19700 23502 +3802
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| if tbl.TempTableType == model.TempTableGlobal { | ||
| if tempTbl := ctx.vars().GetTemporaryTable(tbl); tempTbl != nil { | ||
| if alloc := tempTbl.GetAutoIDAllocator(); alloc != nil { | ||
| return autoid.NewAllocators(false, alloc), true |
There was a problem hiding this comment.
Why autoid.NewAllocators(false? Shouldn't it be autoid.NewAllocators(tbl.SepAutoInc()
What happen for this case, it uses a different autoid key encoding?
create global temporary table t (id int) on commit delete rows AUTO_ID_CACHE=1
There was a problem hiding this comment.
B.t.w, why we did not set Allocators for temp table in the past?
This function call increase object allocation by 1 op/s
There was a problem hiding this comment.
The code is from https://github.com/pingcap/tidb/pull/39041/files#diff-ccb2771ef1931990c0d0ce3703def7dd53bd21d00a8f6b40b59ca0115f776a60R1550
This PR does not change any code logic.
However, I think it's fine for the current code because the id allocation for memory table is pure in memory and does not share with other sessions.
ti-chi-bot
bot
added
lgtm
and removed
needs-1-more-lgtm
[LGTM Timeline notifier]Timeline:
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tangenta, tiancaiamao, YangKeao The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
* upstream/master: (93 commits) disjoinset: add generic impl (pingcap#54917) planner: derive index filters for mv index paths (pingcap#54877) br: cli refactor backup error handling logic (pingcap#54697) expression: fix infinity loop in `timestampadd` (pingcap#54916) planner: import more expand test. (pingcap#54962) planner: use code-gen to generate CloneForPlanCache method for some operators (pingcap#54957) test: fix flaky test TestFailSchemaSyncer (pingcap#54958) planner: move logical show into logicalop pkg. (pingcap#54928) privilege: Remove TestAbnormalMySQLTable (pingcap#54925) resource_control: support unlimited keyword when setting the resource group (pingcap#54704) ddl: fix a data race on localRowCntListener Written() (pingcap#54484) lightning: fix SET SESSION on connection pool (pingcap#54927) planner: move logical mem-table to logicalop pkg. (pingcap#54903) table: Add `CachedTableSupport` and `TemporaryTableSupport` for `MutateContext` (pingcap#54900) executor: fix index_hash_join hang when context canceled (pingcap#54855) statistics: add metrics for unneeded analyze table (pingcap#54822) *: refine pipelined dml benchmarks (pingcap#54844) ddl: assign table IDs for jobs submitted to queue (pingcap#54880) *: avoid using Tables field of model.DBInfo, use API instead (pingcap#52302) planner: remove useless check (pingcap#54907) ...
What problem does this PR solve?
Issue Number: close #54397
What changed and how does it work?
To avoid calling
GetSessionVarsinMutateContext, this PR added two extra methodsGetCachedTableSupportandGetTemporaryTableSupportto handle temporary/cached tables in table operations.CachedTableSupportandGetTemporaryTableSupportare optional and can be nil in some scenes, ie, DDL reorg and lightning import.This PR also changes the only method in
AllocatorContexttoAlternativeAllocatorsto return alternative allocators in context when allocating id. Now, it is only used by temporary table.Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.