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

Fix ingest write amplification after snapshot apply optimization #1913

Merged
merged 2 commits into from
May 18, 2021
Merged

Fix ingest write amplification after snapshot apply optimization #1913

merged 2 commits into from
May 18, 2021

Conversation

JaySon-Huang
Copy link
Contributor

@JaySon-Huang JaySon-Huang commented May 17, 2021 •
edited
Loading

What problem does this PR solve?

Issue Number: A following patch after #1439

Report the bytes written by decoding SSTFiles into DTFiles as a part of ProfileEvents::DMWriteBytes to calculate the write amplification.

What is changed and how it works?

Update ProfileEvents::DMWriteBytes after DTFiles are generated successfully.

Related changes

  • Need to cherry-pick to the release branch: release-5.0

Check List

Tests

  • Manual test (add detailed scripts or steps below)

Side effects

Release note

  • No release note

@JaySon-Huang JaySon-Huang requested a review from flowbehappy May 17, 2021 07:23
@JaySon-Huang JaySon-Huang self-assigned this May 17, 2021
@JaySon-Huang JaySon-Huang added the type/bugfix This PR fixes a bug. label May 17, 2021
@JaySon-Huang JaySon-Huang force-pushed the fix_ingest_write_amp branch from f825e3d to 3157409 Compare May 17, 2021 09:42
@JaySon-Huang
Copy link
Contributor Author

/run-all-tests

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label May 18, 2021
@JaySon-Huang JaySon-Huang force-pushed the fix_ingest_write_amp branch from 73fa63d to efe029c Compare May 18, 2021 08:39
@JaySon-Huang
Copy link
Contributor Author

JaySon-Huang commented May 18, 2021 •
edited
Loading

Add a commit to fix a bug caused by tests/delta-merge-test/raft/schema/drop_on_restart.test under CI env.
The db4.test created in drop_on_restart make trouble for other tests because other tests create default.test but share the same data path of db4.test.
Physically clean up those databases and tables after drop_on_restart.

The log for the fail test case: ci_test_fail.log

@JaySon-Huang
Copy link
Contributor Author

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label May 18, 2021
@ti-srebot
Copy link
Collaborator

/run-all-tests

@ti-srebot ti-srebot merged commit fd01ba5 into pingcap:master May 18, 2021
@JaySon-Huang JaySon-Huang deleted the fix_ingest_write_amp branch May 18, 2021 10:07
JaySon-Huang pushed a commit that referenced this pull request May 18, 2021
@ti-srebot @solotzg
Pre-handle SSTFiles to DTFiles when applying snapshots (#1439) (#1867)
c947fd6 
* Add `raft.snapshot.method` in configuration file to control the method of applying snapshots
  - "block" decode snapshot data as a block
  - "file1" decode snapshot data as DTFiles (directory mode) [**By default**]
* Add a stream `SSTFilesToBlockInputStream` to decode snapshot data into blocks
* Add a stream `BoundedSSTFilesToBlockInputStream` to bound the blocks read from SSTFilesToBlockInputStream by column `_tidb_rowid`
* Add a stream `SSTFilesToDTFilesOutputStream` that accept `BoundedSSTFilesToBlockInputStream` as input stream to write blocks into DTFiles
* Make `STORAGE_FORMAT_CURRENT` to be `STORAGE_FORMAT_V2` by default to support ingest DTFile into DT engine
* Fix the bug that the block decoded from SSTFile is not sorted by primary key and version (#1888)
* Fix panic with feature compaction filter with DTFile
* Fix ingest write amplification after snapshot apply optimization (#1913)

Co-authored-by: Zhigao Tong <[email protected]>
Signed-off-by: JaySon-Huang <[email protected]>
JaySon-Huang pushed a commit to JaySon-Huang/tiflash that referenced this pull request Jun 8, 2021
@ti-srebot @JaySon-Huang
Fix split on empty segment (pingcap#1823)
468cadd 
Signed-off-by: ti-srebot <[email protected]>

Co-authored-by: Flowyi <[email protected]>

cherry pick pingcap#1821 to release-5.0 (pingcap#1827)

Signed-off-by: ti-srebot <[email protected]>

Co-authored-by: xufei <[email protected]>

Remove ReplicatedMergeTree family (pingcap#1805) (pingcap#1826)

cherry pick pingcap#1835 to release-5.0 (pingcap#1840)

Signed-off-by: ti-srebot <[email protected]>

Co-authored-by: xufei <[email protected]>

Use file path as cache key (pingcap#1852) (pingcap#1862)

Lazily initializing DeltaMergeStore  (pingcap#1423) (pingcap#1751) (pingcap#1868)

Add background gc check thread for DeltaTree storage (pingcap#1742) (pingcap#1828)

* cherry pick pingcap#1742 to release-5.0

Signed-off-by: ti-srebot <[email protected]>

* fix conflict

Co-authored-by: lidezhu <[email protected]>

Fix calculate stable property (pingcap#1839) (pingcap#1878)

* cherry pick pingcap#1839 to release-5.0

Signed-off-by: ti-srebot <[email protected]>

* fix compile and cherry pick a small fix

Co-authored-by: lidezhu <[email protected]>

Fix client-c mistake setting current_ts to 0 when resolving lock for async commit (pingcap#1856) (pingcap#1870)

support constant folding in dbg coprocessor (pingcap#1881) (pingcap#1884)

Optimize & Reduce time cost about ci test (pingcap#1849) (pingcap#1894)

* reduce time cost about ci test by parallel
* add `-DNO_WERROR=ON` to cmake config for release-darwin build
* Fix tidb_ghpr_tics_test fail (pingcap#1895)

Signed-off-by: Zhigao Tong <[email protected]>

Fix panic with feature `compaction filter` in release-5.0 (pingcap#1891)

Init store in background task. (pingcap#1843,pingcap#1896) (pingcap#1874)

Push down filter on timestamp type column to storage layer (pingcap#1875) (pingcap#1906)

* cherry pick pingcap#1875 to release-5.0

Signed-off-by: ti-srebot <[email protected]>

* remove irrevalant code

Co-authored-by: lidezhu <[email protected]>

Fix ExchangeSender: remove duplicated write stream operation (pingcap#1379) (pingcap#1883)

cherry pick pingcap#1917 to release-5.0 (pingcap#1923)

Signed-off-by: ti-srebot <[email protected]>

Co-authored-by: Fu Zhe <[email protected]>

Pre-handle SSTFiles to DTFiles when applying snapshots (pingcap#1439) (pingcap#1867)

* Add `raft.snapshot.method` in configuration file to control the method of applying snapshots
  - "block" decode snapshot data as a block
  - "file1" decode snapshot data as DTFiles (directory mode) [**By default**]
* Add a stream `SSTFilesToBlockInputStream` to decode snapshot data into blocks
* Add a stream `BoundedSSTFilesToBlockInputStream` to bound the blocks read from SSTFilesToBlockInputStream by column `_tidb_rowid`
* Add a stream `SSTFilesToDTFilesOutputStream` that accept `BoundedSSTFilesToBlockInputStream` as input stream to write blocks into DTFiles
* Make `STORAGE_FORMAT_CURRENT` to be `STORAGE_FORMAT_V2` by default to support ingest DTFile into DT engine
* Fix the bug that the block decoded from SSTFile is not sorted by primary key and version (pingcap#1888)
* Fix panic with feature compaction filter with DTFile
* Fix ingest write amplification after snapshot apply optimization (pingcap#1913)

Co-authored-by: Zhigao Tong <[email protected]>
Signed-off-by: JaySon-Huang <[email protected]>

Cast int/real as real (pingcap#1911) (pingcap#1928)

Fix the problem that old dmfile is not removed atomically (pingcap#1918) (pingcap#1925)

Eliminate the table lock between reading,writing and DDL operators for TiFlash (pingcap#1736) (pingcap#1858)

* Eliminate the table lock between reading,writing and DDL operators for TiFlash (pingcap#1736)

1. Port the latest `RWLock` for `IStorage` from Clickhouse. "phase-fair rwlocks have been shown to provide more predictable timings compared to task-fair rwlocks for certain workloads (for relatively low share of writes, <25%). Brandenburg B, Anderson J, (2010)"
2. Refactor the lock model for `IStorage` to eliminate the lock between reading, writing, and DDL operations.
3. Acquire table lock by QueryID/threadName instead of function name
4. Tombstone the database after receiving a "drop" DDL from TiDB. Physically drop the databases / tables after gc safe point.
5. Remove some outdated tests under `tests/mutable-test/txn_schema` (all those tests are ported into `tests/delta-merge-test/raft/schema`)

* Mark FIXME for conflict codes
Conflicts:
  dbms/src/Flash/Coprocessor/DAGQueryBlockInterpreter.cpp

Signed-off-by: JaySon-Huang <[email protected]>

cached tics code incase of clone same code repeatedly (pingcap#1994) (pingcap#2001)

Add row & bytes threshold config for CompactLog (pingcap#1997) (pingcap#2005)

* cherry pick pingcap#1997 to release-5.0

Signed-off-by: ti-srebot <[email protected]>
Co-authored-by: Zhigao Tong <[email protected]>

Fix the bug that incomplete write batches are not truncated (pingcap#1934) (pingcap#2003)

Revert "Push down filter on timestamp type column to storage layer (pingcap#1875) (pingcap#1906)" (pingcap#2010)

This reverts commit 630b612.

Revert Lazily Init Store (pingcap#2011)

* Revert "Init store in background task. (pingcap#1843,pingcap#1896) (pingcap#1874)"

This reverts commit 0882461.

Conflicts:
	dbms/src/Storages/StorageDeltaMerge.cpp

* format code

* Revert "Lazily initializing DeltaMergeStore  (pingcap#1423) (pingcap#1751) (pingcap#1868)"

This reverts commit bbce050.

Conflicts:
	dbms/src/Storages/DeltaMerge/tests/gtest_dm_storage_delta_merge.cpp
	dbms/src/Storages/StorageDeltaMerge.cpp
	dbms/src/Storages/StorageDeltaMerge.h

Co-authored-by: JinheLin <[email protected]>
Co-authored-by: Flowyi <[email protected]>

Revert "background gc thread" (pingcap#2015)

* Revert "Fix calculate stable property (pingcap#1839) (pingcap#1878)"

This reverts commit 6a9eb58.

* Revert "Add background gc check thread for DeltaTree storage (pingcap#1742) (pingcap#1828)"

This reverts commit cbbbd09.

* remove code

* format code

* format code

* fix test compile

* fix comment

Fix the potential concurrency problem when clone the shared delta index (pingcap#2030) (pingcap#2033)

* cherry pick pingcap#2030 to release-5.0

Signed-off-by: ti-srebot <[email protected]>

* Update dbms/src/Storages/DeltaMerge/DeltaIndex.h

Co-authored-by: JaySon <[email protected]>

Co-authored-by: lidezhu <[email protected]>
Co-authored-by: JaySon <[email protected]>

Fix bug for unexpected deleted ingest file (pingcap#2047) (pingcap#2048)

* Only enable GC for DTFiles after they get applied to all segments.
* Refine some loggings

Signed-off-by: JaySon-Huang <[email protected]>

Fix cast int as time (pingcap#1788) (pingcap#1893)

cherry pick pingcap#1903 to release-5.0 (pingcap#1915)

Signed-off-by: ti-srebot <[email protected]>

Fix ingesting file may add ref page to deleted page (pingcap#2054) (pingcap#2055)

1. Put the ingest file id into `storage_pool.data` before ingesting them into segments
2. Add ref pages to the ingest files for each segment
3. Delete the original page id after all

Signed-off-by: JaySon-Huang <[email protected]>

Fix compile error on Mac Clang 12.0.5 (pingcap#2058) (pingcap#2070)

cherry pick pingcap#2017 pingcap#2020 pingcap#2051 to release-5.0 (pingcap#2074)

Revert "cherry pick pingcap#2017 pingcap#2020 pingcap#2051 to release-5.0 (pingcap#2074)"

This reverts commit 8e0712c.

Revert "Fix compile error on Mac Clang 12.0.5 (pingcap#2058) (pingcap#2070)"

This reverts commit 3c31b92.

Revert "Fix ingesting file may add ref page to deleted page (pingcap#2054) (pingcap#2055)"

This reverts commit c773d61.

Revert "Fix bug for unexpected deleted ingest file (pingcap#2047) (pingcap#2048)"

This reverts commit b329618.

Revert "Pre-handle SSTFiles to DTFiles when applying snapshots (pingcap#1439) (pingcap#1867)"

This reverts commit c947fd6.

 Conflicts:
	dbms/src/Common/FailPoint.cpp
	dbms/src/Storages/DeltaMerge/File/DMFileBlockOutputStream.h
	dbms/src/Storages/DeltaMerge/File/DMFileWriter.h
	dbms/src/Storages/DeltaMerge/ReorganizeBlockInputStream.h
	dbms/src/Storages/DeltaMerge/SSTFilesToBlockInputStream.cpp
	dbms/src/Storages/DeltaMerge/SSTFilesToBlockInputStream.h
	dbms/src/Storages/DeltaMerge/SSTFilesToDTFilesOutputStream.cpp
	dbms/src/Storages/DeltaMerge/Segment.cpp
	dbms/src/Storages/DeltaMerge/StableValueSpace.cpp
	dbms/src/Storages/DeltaMerge/tests/gtest_dm_file.cpp
	dbms/src/Storages/DeltaMerge/tests/gtest_dm_storage_delta_merge.cpp
	dbms/src/Storages/StorageDeltaMerge.cpp
	dbms/src/Storages/StorageDeltaMerge.h
	dbms/src/Storages/Transaction/ApplySnapshot.cpp
	dbms/src/Storages/Transaction/PartitionStreams.cpp
	tests/delta-merge-test/raft/schema/drop_on_restart.test

Fix comment

Signed-off-by: JaySon-Huang <[email protected]>
@JaySon-Huang JaySon-Huang mentioned this pull request Jun 8, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flowbehappy flowbehappy flowbehappy approved these changes

Assignees

@JaySon-Huang JaySon-Huang

Labels
status/can-merge Indicates a PR has been approved by a committer. status/LGT1 Indicates that a PR has LGTM 1. type/bugfix This PR fixes a bug.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@JaySon-Huang @ti-srebot @flowbehappy