Refactor Regions Storage #11

solotzg · 2019-03-13T11:39:29Z

store all regions in one partition
refactor read process in MergeTreeDataSelectExecutor:
- select and choose region config from setting.
- check region version.
- select parts and range of marks from CH by range of key in region.
- concurrently generate block input stream, which is based on the granularity of region.
flush data into CH first and then remove it from cache.

By using test data of tpch10, operation load/search/change/delete are correct
TODO

something may be wrong with operation like remove region.

dbms/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp

dbms/src/DataStreams/RangesFilterBlockInputStream.cpp

dbms/src/Storages/Transaction/Region.cpp

dbms/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp

dbms/src/Storages/Transaction/PartitionDataMover.cpp

zanmato1984 · 2019-03-15T04:48:44Z

/run-integration-tests

zanmato1984 · 2019-03-15T04:55:51Z

Please merge latest raft branch to enable ci test.

dbms/src/DataStreams/RangesFilterBlockInputStream.cpp

…o refactor_region

zanmato1984 · 2019-03-15T08:18:22Z

/run-integration-tests

dbms/src/Storages/Transaction/RegionTable.cpp

zanmato1984 · 2019-03-19T06:22:55Z

dbms/src/Storages/Transaction/KVStore.h


-    // For test, please do NOT remove.
-    RegionMap & _regions() { return regions; }
+    RegionMap getRegions();


This function returns value rather than const reference. Is this by design?

I've fix it now by making it return const reference.

dbms/src/Storages/Transaction/KVStore.cpp

innerr · 2019-03-19T09:14:30Z

dbms/src/DataStreams/RangesFilterBlockInputStream.cpp

+
+        size_t rows = block.rows();
+
+        auto handle_bg = column->getElement(0);


bg => begin, ed => end

innerr · 2019-03-19T09:24:32Z

dbms/src/Debug/dbgFuncRegion.cpp

@@ -222,16 +219,15 @@ void dbgFuncRegionPartition(Context & context, const ASTs & args, DBGInvoker::Pr
            }

            std::stringstream ss;
-            ss << "table #" << table_id << " partition #" << partition_id <<
-                " region #" << region_id << region_range_info.str();
+            ss << "table #" << table_id << " region #" << region_id << region_range_info.str();


Dump more region status: region meta, raft log index, etc

innerr · 2019-03-19T09:36:14Z

dbms/src/Interpreters/InterpreterSelectQuery.cpp

@@ -663,6 +666,24 @@ QueryProcessingStage::Enum InterpreterSelectQuery::executeFetchColumns(Pipeline
        query_info.resolve_locks = settings.resolve_locks;
        query_info.read_tso = settings.read_tso;

+        String region_str = settings.region;


One region each query?

@hanfei1991

zanmato1984 · 2019-03-19T09:37:21Z

dbms/src/Storages/Transaction/KVStore.cpp

-            region_persister.persist(curr_region);
-            for (const auto & region : split_regions)
-                region_persister.persist(region);
+            if (tmt_ctx)


Any reason swapping the order of persisting regions and calling region_table.splitRegion?

data in kvstore must be stored first.

zanmato1984 · 2019-03-19T09:38:09Z

dbms/src/Storages/Transaction/KVStore.cpp

            {
                std::lock_guard<std::mutex> lock(mutex);

-                curr_region->swap(*new_region);
+                regions[curr_region_id] = new_region;


Any reason use two assignment to replace calling swap?

I've propose a better solution. The original function swap also has bugs. 6528907#diff-b97987fcf9f8bc8e8506a0b07f2512edR285

zanmato1984 · 2019-03-19T11:27:26Z

dbms/src/Storages/Transaction/PartitionStreams.cpp

-            partition_regions.push_back(region);
-        }
+    if (region_version != InvalidRegionVersion && (region->version() != region_version || region->conf_ver() != conf_version)) {
+        std::cout <<"for region "<< region_id <<", the version in ch is "<< region_version << " and "<< conf_version <<" , but the required version is " << region->version() <<" and "<< region->conf_ver() <<".\n";


Why use std::cout?

The message content is kind of confusing: what do you mean by "version in ch" and "required version"? Is NOT region->version() the "version in ch"? Is not passed in region_version the "required version"?

@hanfei1991

zanmato1984 · 2019-03-19T11:38:54Z

dbms/src/Storages/Transaction/RegionException.h

+namespace DB
+{
+
+class RegionException : public Exception


Shall we record the exception type in this class? Outer may want to know if this exception is caused by NOT_FOUND or VERSION_ERROR.

@hanfei1991

innerr · 2019-03-19T16:40:45Z

dbms/src/Storages/Transaction/KVStore.cpp

@@ -64,8 +68,8 @@ void KVStore::onSnapshot(const RegionPtr & region, Context * context)
            old_region = it->second;
    }

-    if (tmt_ctx && old_region)
-        tmt_ctx->region_partition.removeRegion(old_region);
+    if (old_region != nullptr && old_region->getIndex() >= region->getIndex())


What for old_region->getIndex() >= region->getIndex()?

We don't need outdated region.

innerr · 2019-03-19T16:42:25Z

dbms/src/Storages/Transaction/KVStore.cpp

            {
                std::lock_guard<std::mutex> lock(mutex);

-                curr_region->swap(*new_region);
+                regions[curr_region_id] = new_region;


@zanmato1984 Plz take a look to confirm this modification

I've propose a better solution. The original function swap also has bugs.

dbms/src/Storages/Transaction/RegionMeta.cpp

zanmato1984 · 2019-03-20T08:55:09Z

/run-integration-tests

zanmato1984 · 2019-03-20T09:00:27Z

LGTM

solotzg added 10 commits March 13, 2019 01:14

refactor region;

b036d48

Fix Bug.

9d13ff6

Optimize code.

d832282

Remove meaningless log in region.

b9990fd

Small fix.

a66708e

tmp

0923fdc

fix

4d97f2e

Fix.

8dae908

Optimize code.

b28daeb

Rename RegionPartition to RegionTable;

176abcd

solotzg commented Mar 13, 2019

View reviewed changes

dbms/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp Show resolved Hide resolved

Reformat code; Optimize.

f0afbcf

zanmato1984 reviewed Mar 14, 2019

View reviewed changes

dbms/src/DataStreams/RangesFilterBlockInputStream.cpp Outdated Show resolved Hide resolved

Fix.

6195db7

zanmato1984 reviewed Mar 14, 2019

View reviewed changes

dbms/src/Storages/Transaction/Region.cpp Outdated Show resolved Hide resolved

solotzg added 3 commits March 14, 2019 18:35

Fix: pk may not be _tidb_rowid.

bfdefde

Rename getRegionRangeField to getHandleRangeByTable.

fa95c85

small fix.

9fad4cc

innerr reviewed Mar 14, 2019

View reviewed changes

dbms/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp Show resolved Hide resolved

innerr reviewed Mar 14, 2019

View reviewed changes

dbms/src/Storages/Transaction/PartitionDataMover.cpp Outdated Show resolved Hide resolved

solotzg added 2 commits March 15, 2019 10:42

rollback moveRangeBetweenPartitions.

c56db7e

reformat code.

c7769a0

solotzg changed the title ~~[Don't Merge Now] Refactor Regions Storage~~ Refactor Regions Storage Mar 15, 2019

Optimize KVStore::tryPersistAndReport.

ffe56d3

Optimize

5f69a6f

zanmato1984 reviewed Mar 15, 2019

View reviewed changes

dbms/src/DataStreams/RangesFilterBlockInputStream.cpp Show resolved Hide resolved

zanmato1984 added 2 commits March 15, 2019 15:42

Fix pointing to on-stack variables

a68b76f

Merge branch 'refactor_region' of https://github.com/solotzg/tics int…

bd7890f

…o refactor_region

Merge branch 'raft' into refactor_region

405ea15

zanmato1984 reviewed Mar 15, 2019

View reviewed changes

dbms/src/Storages/Transaction/RegionTable.cpp Show resolved Hide resolved

zanmato1984 reviewed Mar 19, 2019

View reviewed changes

dbms/src/Storages/Transaction/KVStore.cpp Outdated Show resolved Hide resolved

zanmato1984 reviewed Mar 19, 2019

View reviewed changes

dbms/src/Storages/Transaction/KVStore.cpp Outdated Show resolved Hide resolved

innerr reviewed Mar 19, 2019

View reviewed changes

zanmato1984 reviewed Mar 19, 2019

View reviewed changes

innerr reviewed Mar 19, 2019

View reviewed changes

refactor Region::splitInto.

6528907

zanmato1984 reviewed Mar 20, 2019

View reviewed changes

dbms/src/Storages/Transaction/RegionMeta.cpp Show resolved Hide resolved

solotzg added 2 commits March 20, 2019 16:43

Optimize select no_kvstore.

92aa461

remove unused code.

9bc5e42

solotzg merged commit 8239285 into pingcap:raft Mar 20, 2019

solotzg deleted the refactor_region branch March 23, 2019 12:50

hehechen mentioned this pull request Jul 6, 2022

AddressSanitizer: stack-use-after-scope in ExecutorTopNTestRunner.TopN and ExecutorTopNTestRunner.TopNFunction #5298

Closed

bestwoody mentioned this pull request Sep 13, 2022

tsan warnings of accuracy_diff_for_test #5853

Closed

hehechen mentioned this pull request Sep 26, 2022

SyncStatusTest UT detected memory leaks #6025

Closed

CalvinNeo mentioned this pull request Jun 15, 2023

[WIP] Proactive flush of delta merge layer phase 1 #7558

Closed

12 tasks

yibin87 mentioned this pull request Oct 12, 2023

Tsan fail: ComputeServerRunner.testMinTSOActiveSetSoftLimit #7774

Open

yibin87 mentioned this pull request Nov 1, 2023

ASAN/TSan failed compute test case #8285

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor Regions Storage #11

Refactor Regions Storage #11

solotzg commented Mar 13, 2019 •

edited

Loading

zanmato1984 commented Mar 15, 2019

zanmato1984 commented Mar 15, 2019

zanmato1984 commented Mar 15, 2019

zanmato1984 Mar 19, 2019

solotzg Mar 20, 2019 •

edited

Loading

innerr Mar 19, 2019

solotzg Mar 20, 2019

innerr Mar 19, 2019

innerr Mar 19, 2019

solotzg Mar 19, 2019

zanmato1984 Mar 19, 2019 •

edited

Loading

solotzg Mar 20, 2019

zanmato1984 Mar 19, 2019

solotzg Mar 20, 2019

zanmato1984 Mar 19, 2019

solotzg Mar 19, 2019

zanmato1984 Mar 19, 2019

solotzg Mar 19, 2019

innerr Mar 19, 2019

solotzg Mar 20, 2019

innerr Mar 19, 2019

solotzg Mar 20, 2019

zanmato1984 commented Mar 20, 2019

zanmato1984 commented Mar 20, 2019


		size_t rows = block.rows();

		auto handle_bg = column->getElement(0);

Refactor Regions Storage #11

Refactor Regions Storage #11

Conversation

solotzg commented Mar 13, 2019 • edited Loading

zanmato1984 commented Mar 15, 2019

zanmato1984 commented Mar 15, 2019

zanmato1984 commented Mar 15, 2019

Choose a reason for hiding this comment

solotzg Mar 20, 2019 • edited Loading

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

zanmato1984 Mar 19, 2019 • edited Loading

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

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

zanmato1984 commented Mar 20, 2019

zanmato1984 commented Mar 20, 2019

solotzg commented Mar 13, 2019 •

edited

Loading

solotzg Mar 20, 2019 •

edited

Loading

zanmato1984 Mar 19, 2019 •

edited

Loading