Refactor Regions Storage#11
Conversation
|
/run-integration-tests |
|
Please merge latest raft branch to enable ci test. |
|
/run-integration-tests |
|
|
||
| // For test, please do NOT remove. | ||
| RegionMap & _regions() { return regions; } | ||
| RegionMap getRegions(); |
There was a problem hiding this comment.
This function returns value rather than const reference. Is this by design?
There was a problem hiding this comment.
I've fix it now by making it return const reference.
dbms/src/Debug/dbgFuncRegion.cpp
Outdated
There was a problem hiding this comment.
Dump more region status: region meta, raft log index, etc
There was a problem hiding this comment.
Any reason swapping the order of persisting regions and calling region_table.splitRegion?
There was a problem hiding this comment.
data in kvstore must be stored first.
There was a problem hiding this comment.
Any reason use two assignment to replace calling swap?
There was a problem hiding this comment.
I've propose a better solution. The original function swap also has bugs. 6528907#diff-b97987fcf9f8bc8e8506a0b07f2512edR285
There was a problem hiding this comment.
- 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 inregion_versionthe "required version"?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
What for old_region->getIndex() >= region->getIndex()?
There was a problem hiding this comment.
We don't need outdated region.
There was a problem hiding this comment.
@zanmato1984 Plz take a look to confirm this modification
There was a problem hiding this comment.
I've propose a better solution. The original function swap also has bugs.
|
/run-integration-tests |
|
LGTM |
store all regions in one partition
refactor read process in MergeTreeDataSelectExecutor:
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
remove region.