lightning: support compress for lightning, add compress unit and integration tests #39153

lichunzhu · 2022-11-15T07:15:19Z

What problem does this PR solve?

Issue Number: ref #38514

Problem Summary:

What is changed and how it works?

Support file reader for schema files.
Add missing usage for sample functions.
Add more unit tests and integration tests for compress lightning.

Check List

Tests

Unit test
Integration test

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Affects user behaviors

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

lightning support importing compressed sql/csv files

ti-chi-bot · 2022-11-15T07:15:20Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

buchuitoudegou
dsdashun

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

…into lightningCompress

lichunzhu · 2022-11-17T03:39:51Z

/run-integration-br-test

buchuitoudegou · 2022-11-18T07:53:51Z

br/pkg/lightning/mydump/region.go

@@ -34,6 +34,8 @@ const (
 	tableRegionSizeWarningThreshold int64 = 1024 * 1024 * 1024
 	// the increment ratio of large CSV file size threshold by `region-split-size`
 	largeCSVLowerThresholdRation = 10
+	// TableFileSizeINF for compressed size, for lightning 10TB is a relatively big value and will strongly affect efficiency
+	TableFileSizeINF = 10 * 1024 * tableRegionSizeWarningThreshold


What does INF mean? infinite?

Yes, INF means infinite. It's used to make sure compressed files can be read until the EOF error.

set fileSize to INF to make sure compressed files can be read until EOF. Because we can't get the exact size of the compressed files.

I would expect to put its usage in the comment (as well as why this works, maybe)

addressed in 46ddf53

buchuitoudegou · 2022-11-18T08:39:54Z

br/pkg/lightning/mydump/router.go

@@ -324,6 +324,9 @@ func (p regexRouterParser) Parse(r *config.FileRouteRule, logger log.Logger) (*R
 			if err != nil {
 				return err
 			}
+			if result.Type == SourceTypeParquet && compression != CompressionNone {
+				return errors.Errorf("can't support whole compressed parquet file, should compress parquet files by choosing correct parquet compress writer, path: %s", r.Path)


What does this workaround mean here? I remember we support compressed parquet files.

We support compressed binary data in parquet files, but we don't support compressed parquet files like parquet.gz.

buchuitoudegou · 2022-11-18T08:51:26Z

br/pkg/lightning/restore/get_pre_info.go

+		if err != nil {
+			break
+		}
+		reader, err = storage.WithCompression(p.srcStorage, compressType).Open(ctx, sampleFile.Path)


Duplicate with reader.go too. Maybe we should encapsulate this with a function.

addressed in 4c7eaf9

buchuitoudegou · 2022-11-18T09:12:08Z

/component lightning

lichunzhu · 2022-11-23T08:08:32Z

@buchuitoudegou @dsdashun PTAL

buchuitoudegou

rest lgtm

buchuitoudegou · 2022-11-23T08:28:51Z

br/pkg/lightning/mydump/region.go

+	// set fileSize to INF to make sure compressed files can be read until EOF. Because we can't get the exact size of the compressed files.
+	// TODO: update progress bar calculation for compressed files.
+	if fi.FileMeta.Compression != CompressionNone {
+		rowIDMax = fileSize * 100 / divisor


I assume we should do some tests on it. It seems this estimate can easily allocate too many row ids and make lightning fail to insert rows that need auto id even when the number of rows is far smaller than the type limit.

leave a fixme in 46ddf53
I will do some tests and maybe fix this problem in the next PR.

buchuitoudegou · 2022-11-23T08:29:44Z

br/pkg/lightning/mydump/region.go

@@ -34,6 +34,8 @@ const (
 	tableRegionSizeWarningThreshold int64 = 1024 * 1024 * 1024
 	// the increment ratio of large CSV file size threshold by `region-split-size`
 	largeCSVLowerThresholdRation = 10
+	// TableFileSizeINF for compressed size, for lightning 10TB is a relatively big value and will strongly affect efficiency
+	TableFileSizeINF = 10 * 1024 * tableRegionSizeWarningThreshold


set fileSize to INF to make sure compressed files can be read until EOF. Because we can't get the exact size of the compressed files.

I would expect to put its usage in the comment (as well as why this works, maybe)

buchuitoudegou

lgtm

dsdashun

rest LGTM

dsdashun · 2022-11-28T07:22:24Z

br/pkg/lightning/mydump/region.go

 		},
 	}

-	if tableRegion.Size() > tableRegionSizeWarningThreshold {
+	if fi.FileMeta.Compression == CompressionNone && tableRegion.Size() > tableRegionSizeWarningThreshold {


If a compressed file's size exceeds the threshold, the actual size is even bigger. So the threshold check is suitable for both compressed and uncompressed file ? Otherwise, the compressed file won't receive this warning message.

addressed in a3ce953

…into lightningCompress

dsdashun

LGTM

lichunzhu · 2022-11-28T08:53:14Z

/merge

ti-chi-bot · 2022-11-28T08:53:18Z

This pull request has been accepted and is ready to merge.

Commit hash: a91ad73

sre-bot · 2022-11-28T11:24:47Z

TiDB MergeCI notify

🔴 Bad News! New failing [1] after this pr merged.
These new failed integration tests seem to be caused by the current PR, please try to fix these new failed integration tests, thanks!

CI Name	Result	Duration	Compare with Parent commit
idc-jenkins-ci-tidb/integration-common-test	🟥 failed 1, success 16, total 17	18 min	New failing
idc-jenkins-ci/integration-cdc-test	🟢 all 40 tests passed	20 min	Existing passed
idc-jenkins-ci-tidb/common-test	🟢 all 11 tests passed	9 min 53 sec	Existing passed
idc-jenkins-ci-tidb/sqllogic-test-2	🟢 all 28 tests passed	7 min 37 sec	Existing passed
idc-jenkins-ci-tidb/sqllogic-test-1	🟢 all 26 tests passed	6 min 56 sec	Existing passed
idc-jenkins-ci-tidb/mybatis-test	🟢 all 1 tests passed	6 min 44 sec	Existing passed
idc-jenkins-ci-tidb/tics-test	🟢 all 1 tests passed	6 min 44 sec	Existing passed
idc-jenkins-ci-tidb/integration-ddl-test	🟢 all 6 tests passed	5 min 5 sec	Existing passed
idc-jenkins-ci-tidb/integration-compatibility-test	🟢 all 1 tests passed	2 min 56 sec	Existing passed
idc-jenkins-ci-tidb/plugin-test	🟢 build success, plugin test success	4min	Existing passed

support compress for lightning, add compress reader tests

e0ed623

ti-chi-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 15, 2022

lichunzhu added 9 commits November 15, 2022 15:16

Merge branch 'master' into lightningCompress

f4c9873

fix tests

1fa5b1a

Merge branch 'lightningCompress' of https://github.com/lichunzhu/tidb …

7a07c2d

…into lightningCompress

Merge branch 'master' into lightningCompress

39e30db

fix lightning/dumpling integration tests

2238172

Merge branch 'lightningCompress' of https://github.com/lichunzhu/tidb …

dab5dc5

…into lightningCompress

Merge branch 'master' into lightningCompress

90ab390

Merge branch 'master' into lightningCompress

a23bf67

fix ut

e55dd2a

lichunzhu requested review from lance6716 and buchuitoudegou November 17, 2022 03:45

buchuitoudegou reviewed Nov 18, 2022

View reviewed changes

ti-chi-bot added the component/lightning This issue is related to Lightning of TiDB. label Nov 18, 2022

lichunzhu added 2 commits November 18, 2022 17:26

Merge branch 'master' into lightningCompress

536eac5

address comment

4c7eaf9

buchuitoudegou reviewed Nov 23, 2022

View reviewed changes

address comments

46ddf53

buchuitoudegou approved these changes Nov 23, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 23, 2022

dsdashun reviewed Nov 28, 2022

View reviewed changes

lichunzhu added 3 commits November 28, 2022 15:41

address comment

a3ce953

Merge branch 'master' into lightningCompress

9a221d3

address comment

f64281f

lichunzhu added 2 commits November 28, 2022 16:47

Merge branch 'lightningCompress' of https://github.com/lichunzhu/tidb …

369ab8a

…into lightningCompress

Merge branch 'master' into lightningCompress

a91ad73

dsdashun approved these changes Nov 28, 2022

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Nov 28, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Nov 28, 2022

ti-chi-bot added 3 commits November 28, 2022 17:02

Merge branch 'master' into lightningCompress

c8a4817

Merge branch 'master' into lightningCompress

aa1b841

Merge branch 'master' into lightningCompress

9931544

ti-chi-bot merged commit 5b9d96b into pingcap:master Nov 28, 2022

lichunzhu deleted the lightningCompress branch December 2, 2022 06:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lightning: support compress for lightning, add compress unit and integration tests #39153

lightning: support compress for lightning, add compress unit and integration tests #39153

lichunzhu commented Nov 15, 2022

ti-chi-bot commented Nov 15, 2022 •

edited

Loading

lichunzhu commented Nov 17, 2022

buchuitoudegou Nov 18, 2022

lichunzhu Nov 18, 2022

buchuitoudegou Nov 23, 2022

lichunzhu Nov 23, 2022

buchuitoudegou Nov 18, 2022

lichunzhu Nov 18, 2022

buchuitoudegou Nov 18, 2022

lichunzhu Nov 18, 2022

buchuitoudegou commented Nov 18, 2022

lichunzhu commented Nov 23, 2022

buchuitoudegou left a comment

buchuitoudegou Nov 23, 2022

lichunzhu Nov 23, 2022

buchuitoudegou Nov 23, 2022

buchuitoudegou left a comment

dsdashun left a comment

dsdashun Nov 28, 2022

lichunzhu Nov 28, 2022

dsdashun left a comment

lichunzhu commented Nov 28, 2022

ti-chi-bot commented Nov 28, 2022

sre-bot commented Nov 28, 2022

lightning: support compress for lightning, add compress unit and integration tests #39153

lightning: support compress for lightning, add compress unit and integration tests #39153

Conversation

lichunzhu commented Nov 15, 2022

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Nov 15, 2022 • edited Loading

lichunzhu commented Nov 17, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

buchuitoudegou commented Nov 18, 2022

lichunzhu commented Nov 23, 2022

buchuitoudegou left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

buchuitoudegou left a comment

Choose a reason for hiding this comment

dsdashun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dsdashun left a comment

Choose a reason for hiding this comment

lichunzhu commented Nov 28, 2022

ti-chi-bot commented Nov 28, 2022

sre-bot commented Nov 28, 2022

TiDB MergeCI notify

ti-chi-bot commented Nov 15, 2022 •

edited

Loading