lightning, ddl: set TS to engineMeta after ResetEngineSkipAllocTS #57998
Conversation
Signed-off-by: lance6716 <[email protected]>
Signed-off-by: lance6716 <[email protected]>
ti-chi-bot
bot
added
release-note-none
|
Hi @lance6716. 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. |
Signed-off-by: lance6716 <[email protected]>
Signed-off-by: lance6716 <[email protected]>
Signed-off-by: lance6716 <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #57998 +/- ##
================================================
+ Coverage 73.1719% 74.8889% +1.7170%
================================================
Files 1674 1719 +45
Lines 461065 471668 +10603
================================================
+ Hits 337370 353227 +15857
+ Misses 102942 96307 -6635
- Partials 20753 22134 +1381
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| @@ -436,6 +436,10 @@ func (local *Backend) doWrite(ctx context.Context, j *regionJob) error { | |||
| allPeers = append(allPeers, peer) | |||
| } | |||
| dataCommitTS := j.ingestData.GetTS() | |||
| if dataCommitTS == 0 { | |||
| intest.Assert(false, "data commitTS is 0") | |||
There was a problem hiding this comment.
| intest.Assert(false, "data commitTS is 0") |
we have returned error below
There was a problem hiding this comment.
I added it because DDL can retry the whole ingest task. Without this line, the newly added unit test can pass, because when the second task has zero TS causes error, DDL will retry and continues from checkpoint.
There was a problem hiding this comment.
DDL can retry the whole ingest task.
shouldn't this be a fatal error?
Signed-off-by: lance6716 <[email protected]>
Signed-off-by: lance6716 <[email protected]>
|
/hold checking tests |
ti-chi-bot
bot
added
the
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: D3Hunter, tangenta 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 |
ti-chi-bot
bot
added
lgtm
approved
and removed
needs-1-more-lgtm
|
/unhold checked that without the code part changes, the new test |
ti-chi-bot
bot
removed
the
do-not-merge/hold
Signed-off-by: lance6716 <[email protected]>
|
/retest |
|
@lance6716: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
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. |
Signed-off-by: lance6716 <[email protected]>
ti-chi-bot
bot
removed
the
size/M
ti-chi-bot
bot
added
the
size/L
Signed-off-by: lance6716 <[email protected]>
| @@ -448,6 +448,27 @@ func TestAddIndexMockFlushError(t *testing.T) { | |||
| require.True(t, strings.Contains(jobTp, "ingest"), jobTp) | |||
| } | |||
|
|
|||
| func TestAddIndexDiskQuotaTS(t *testing.T) { | |||
There was a problem hiding this comment.
I know add-index with DXF shouldn't have such issue, but can you add a case to cover it too?
There was a problem hiding this comment.
OK, I can add one test for DXF enabled. But we should leave more test improvements in future in a separate PR. Like iterate all variables combinations like {DXF = on/off, disk quota = on/off, ...} and provide a helper function to prepare environment, and run the testing logic will all combinations.
|
/hold I add a new comment, unhold as you wish |
ti-chi-bot
bot
added
the
do-not-merge/hold
Signed-off-by: lance6716 <[email protected]>
|
/unhold |
ti-chi-bot
bot
removed
the
do-not-merge/hold
ti-chi-bot
bot
added
the
needs-cherry-pick-release-8.1
|
In response to a cherrypick label: new pull request created to branch |
Signed-off-by: ti-chi-bot <[email protected]>
ti-chi-bot
bot
added
the
needs-cherry-pick-release-8.5
|
In response to a cherrypick label: new pull request created to branch |
What problem does this PR solve?
Issue Number: close #57980
Problem Summary:
What changed and how does it work?
Previously, after reset engine we set the TS to
EngineConfig. ButengineMetais the place that really take effect. The conversion fromEngineConfigtoengineMetahappens when 1) engine is opened, or 2) external engine is closed normally. In disk-quota triggered import, above conversion does not happenCheck List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.