Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

restore: merge small ranges #578

Merged
merged 25 commits into from
Dec 24, 2020

Conversation

overvenus
Copy link
Member

@overvenus overvenus commented Nov 2, 2020

What problem does this PR solve?

Speed up restore by merging small range to reduce split and scatter.

What is changed and how it works?

Merge adjacent ranges that total_bytes is less 96MB or total_kv is less than 960000.

Check List

Tests

  • Integration test

Side effects

  • Breaking backward compatibility

Release Note

  • Speed up restore by merging small backup files.

@overvenus overvenus force-pushed the debug-optimize-backupmeta branch from 4a34de6 to dd32d46 Compare November 2, 2020 13:38
@overvenus
Copy link
Member Author

/release

@overvenus overvenus force-pushed the debug-optimize-backupmeta branch from dd32d46 to fa4d3a1 Compare November 23, 2020 11:59
@overvenus overvenus changed the base branch from release-4.0 to master November 23, 2020 11:59
@overvenus overvenus marked this pull request as ready for review November 23, 2020 12:01
Signed-off-by: Neil Shen <[email protected]>
@overvenus
Copy link
Member Author

/rebuild

Copy link
Collaborator

@kennytm kennytm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rest LGTM

Comment on lines 22 to 30
run_sql "split table $DB.$TABLE by ('user6282602628620641459'), ('user6282603728132269670'), ('user6282604827643897881'), ('user6282610325202038936');"

run_sql "CREATE TABLE $DB.$TABLE( \
YCSB_KEY varchar(64) NOT NULL, \
FIELD0 varchar(1) DEFAULT NULL, \
PRIMARY KEY (YCSB_KEY) \
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin;"

run_sql "INSERT INTO $DB.$TABLE VALUES (\"a\", \"b\");"
run_sql "INSERT INTO $DB.$TABLE VALUES (\"aa\", \"b\");"
table_region_sql="SELECT COUNT(*) FROM information_schema.tikv_region_status WHERE db_name = '$DB' AND table_name = '$TABLE';"
for i in $(seq 10); do
regioncount=$(run_sql "$table_region_sql" | awk '/COUNT/{print $2}')
[ $regioncount -ge 5 ] && break
sleep 3
done
run_sql "$table_region_sql"
Copy link
Collaborator

@kennytm kennytm Nov 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SPLIT TABLE should be effective immediately, so no need to check region count again 🤔 (you could check the TOTAL_SPLIT_REGION field returned by SPLIT TABLE if you need to be absolutely sure)

btw there is also the SPLIT TABLE a BETWEEN ('aaaa') AND ('zzzz') REGIONS 5 syntax.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does split, however, data is not distributed to multiple regions, so I think just leave the loop and let TiKV splits.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok.

Signed-off-by: Neil Shen <[email protected]>
@overvenus
Copy link
Member Author

/rebuild

@overvenus
Copy link
Member Author

/run-integration-test

Fails with #641

start ts:421372453438095363 is greater than resolved ts:0: [BR:Restore:ErrRestoreResolvedTsConstrain]resolved ts constrain violation

@overvenus
Copy link
Member Author

/run-integration-test

2 similar comments
@kennytm
Copy link
Collaborator

kennytm commented Dec 8, 2020

/run-integration-test

@kennytm
Copy link
Collaborator

kennytm commented Dec 9, 2020

/run-integration-test

@ti-srebot ti-srebot added the status/LGT1 LGTM1 label Dec 23, 2020
overvenus and others added 2 commits December 24, 2020 13:25
Signed-off-by: Neil Shen <[email protected]>
@3pointer
Copy link
Collaborator

/rebuild

@3pointer
Copy link
Collaborator

LGTM

@ti-srebot ti-srebot removed the status/LGT1 LGTM1 label Dec 24, 2020
@ti-srebot ti-srebot added the status/LGT2 LGTM2 label Dec 24, 2020
@glorv
Copy link
Collaborator

glorv commented Dec 24, 2020

/merge

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot ti-srebot merged commit 1a527fb into pingcap:master Dec 24, 2020
ti-srebot pushed a commit to ti-srebot/br that referenced this pull request Dec 24, 2020
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #655

@overvenus
Copy link
Member Author

/run-cherry-picker

@ti-srebot
Copy link
Contributor

cherry pick to release-5.0-rc failed

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants