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.

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

[FLASH-461/471] Basic read/write StorageDeltaMerge's table through Raft #217

Merged
merged 21 commits into from
Sep 12, 2019

Conversation

JaySon-Huang
Copy link
Contributor

@JaySon-Huang JaySon-Huang commented Sep 3, 2019

Changes

  • Add enum StorageEngine for indicating which storage engine to use

  • DBGInvoke mock_tidb_table now support Engine=DeltaMerge

  • We can specify default storage engine by raft.storage_engine in tiflash.xml

  • Use IManageableStorage as interface for Storages synced from TiDB

  • TMTStorages now store ptr to IManageableStore instead of StorageMergeTree

  • Add StorageDeltaMerge::deleteRange

  • Support applySnapshot by using StorageDeltaMerge::deleteRange

  • Use ::DB::MutableSupport for constant naming in DeltaMergeDefines.h

  • Note that we can NOT read data in KVStore by now, we must flush data to StorageDeltaMerge by using some commands like DBGInvoke try_flush_region

Steps to debug for DeltaMerge table

See tests/delta_merge/raft/sync_table_from_raft.test for detail

  • Create a DeltaMerge table and bind to a region
    • using mock_tidb_table, refresh_schemas, put_region
  • Insert data and flush to DM table
    • using raft_insert_row, try_flush_region
  • Select data from DM table as normal

TODO

  1. Support UInt64 handle type. ApplySnapshot.cpp (FLASH-392) @marsishandsome
    • Ensure that getHandleMapByRange<Int64> can handle all non UInt64 pk-type
    • Inside RegionDataMover::getHandleMapByRange, call createBlockInputStreamFromRange. Is DeltaMerge table support SELRAW NOKVSTORE ... ? This is only called by TMT.
  2. Flush data in KVStore to StorageDeltaMerge (FLASH-463)
  3. Read with sepcify schema version (FLASH-469)
  4. Learner read? (FLASH-470)

5. Bug: Try to lock mutex after mutex is free (FLASH-468)
Done @ b00a3a1
6. Bug: If write throw exception, that region can not write any more
Done @ b054cdf, merged to master in this commit 1509cfe

@@ -208,6 +208,7 @@ void InterpreterSelectQuery::getAndLockStorageWithSchemaVersion(const String & d
if (!storage_)
return std::make_tuple(nullptr, nullptr, DEFAULT_UNSPECIFIED_SCHEMA_VERSION, false);

// TODO handle if storage_ is a DeltaMerge?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FLASH-469


static const ColumnDefine VERSION_COLUMN_DEFINE{VERSION_COLUMN_ID, VERSION_COLUMN_NAME, VERSION_COLUMN_TYPE};
static const ColumnDefine TAG_COLUMN_DEFINE{TAG_COLUMN_ID, TAG_COLUMN_NAME, TAG_COLUMN_TYPE};
inline ColumnDefine getVersionColumnDefine()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use static object?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Column name and type are defined in class ::DB::MutableSupport now, and C++ don't ensure the order of init for static object in different compile unit.
If we use static object here, we will get empty column name and nullptr as column type.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe static local object? Like

inline const ColumnDefine & EXTRA_HANDLE_COLUMN()
{
static ColumnDefine VERSION_COLUMN_DEFINE_{VERSION_COLUMN_ID, VERSION_COLUMN_NAME, VERSION_COLUMN_TYPE};
return VERSION_COLUMN_DEFINE_;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@JaySon-Huang
Copy link
Contributor Author

/run-integration-tests

@JaySon-Huang JaySon-Huang changed the title Basic read/write StorageDeltaMerge's table through Raft [FLASH-461/471] Basic read/write StorageDeltaMerge's table through Raft Sep 5, 2019
@JaySon-Huang
Copy link
Contributor Author

/run-integration-tests

1 similar comment
@JaySon-Huang
Copy link
Contributor Author

/run-integration-tests

@JaySon-Huang
Copy link
Contributor Author

/run-integration-tests

1 similar comment
@JaySon-Huang
Copy link
Contributor Author

/run-integration-tests

@JaySon-Huang
Copy link
Contributor Author

/rebuild

1 similar comment
@JaySon-Huang
Copy link
Contributor Author

/rebuild

@JaySon-Huang
Copy link
Contributor Author

/run-integration-tests

@flowbehappy
Copy link
Contributor

LGTM

@JaySon-Huang
Copy link
Contributor Author

/run-integration-tests

@JaySon-Huang JaySon-Huang merged commit 22df6d3 into pingcap:DeltaMergeEngine Sep 12, 2019
JaySon-Huang added a commit to JaySon-Huang/tiflash that referenced this pull request Sep 12, 2019
…ft (pingcap#217)

## Changes

* Add enum StorageEngine for indicating which storage engine to use
* `DBGInvoke mock_tidb_table` now support Engine=DeltaMerge
* We can specify default storage engine by raft.storage_engine in tiflash.xml

* Use `IManageableStorage` as interface for Storages synced from TiDB
* TMTStorages now store ptr to IManageableStore instead of StorageMergeTree

* Add `StorageDeltaMerge::deleteRange`
* Support `applySnapshot` by using `StorageDeltaMerge::deleteRange`

* Use ::DB::MutableSupport for constant naming in DeltaMergeDefines.h

* Note that we can NOT read data in KVStore by now, we must flush data to StorageDeltaMerge by using some commands like `DBGInvoke try_flush_region`
JaySon-Huang added a commit to JaySon-Huang/tiflash that referenced this pull request Sep 18, 2019
…ft (pingcap#217)

## Changes

* Add enum StorageEngine for indicating which storage engine to use
* `DBGInvoke mock_tidb_table` now support Engine=DeltaMerge
* We can specify default storage engine by raft.storage_engine in tiflash.xml

* Use `IManageableStorage` as interface for Storages synced from TiDB
* TMTStorages now store ptr to IManageableStore instead of StorageMergeTree

* Add `StorageDeltaMerge::deleteRange`
* Support `applySnapshot` by using `StorageDeltaMerge::deleteRange`

* Use ::DB::MutableSupport for constant naming in DeltaMergeDefines.h

* Note that we can NOT read data in KVStore by now, we must flush data to StorageDeltaMerge by using some commands like `DBGInvoke try_flush_region`
JaySon-Huang added a commit to JaySon-Huang/tiflash that referenced this pull request Sep 23, 2019
…ft (pingcap#217)

## Changes

* Add enum StorageEngine for indicating which storage engine to use
* `DBGInvoke mock_tidb_table` now support Engine=DeltaMerge
* We can specify default storage engine by raft.storage_engine in tiflash.xml

* Use `IManageableStorage` as interface for Storages synced from TiDB
* TMTStorages now store ptr to IManageableStore instead of StorageMergeTree

* Add `StorageDeltaMerge::deleteRange`
* Support `applySnapshot` by using `StorageDeltaMerge::deleteRange`

* Use ::DB::MutableSupport for constant naming in DeltaMergeDefines.h

* Note that we can NOT read data in KVStore by now, we must flush data to StorageDeltaMerge by using some commands like `DBGInvoke try_flush_region`
JaySon-Huang added a commit to JaySon-Huang/tiflash that referenced this pull request Sep 23, 2019
…ft (pingcap#217)

* Add enum StorageEngine for indicating which storage engine to use
* `DBGInvoke mock_tidb_table` now support Engine=DeltaMerge
* We can specify default storage engine by raft.storage_engine in tiflash.xml

* Use `IManageableStorage` as interface for Storages synced from TiDB
* TMTStorages now store ptr to IManageableStore instead of StorageMergeTree

* Add `StorageDeltaMerge::deleteRange`
* Support `applySnapshot` by using `StorageDeltaMerge::deleteRange`

* Use ::DB::MutableSupport for constant naming in DeltaMergeDefines.h

* Note that we can NOT read data in KVStore by now, we must flush data to StorageDeltaMerge by using some commands like `DBGInvoke try_flush_region`
JaySon-Huang added a commit that referenced this pull request Sep 26, 2019
…ft (#217)

* Add enum StorageEngine for indicating which storage engine to use
* `DBGInvoke mock_tidb_table` now support Engine=DeltaMerge
* We can specify default storage engine by raft.storage_engine in tiflash.xml

* Use `IManageableStorage` as interface for Storages synced from TiDB
* TMTStorages now store ptr to IManageableStore instead of StorageMergeTree

* Add `StorageDeltaMerge::deleteRange`
* Support `applySnapshot` by using `StorageDeltaMerge::deleteRange`

* Use ::DB::MutableSupport for constant naming in DeltaMergeDefines.h

* Note that we can NOT read data in KVStore by now, we must flush data to StorageDeltaMerge by using some commands like `DBGInvoke try_flush_region`
JaySon-Huang added a commit to JaySon-Huang/tiflash that referenced this pull request Oct 17, 2019
…ft (pingcap#217)

* Add enum StorageEngine for indicating which storage engine to use
* `DBGInvoke mock_tidb_table` now support Engine=DeltaMerge
* We can specify default storage engine by raft.storage_engine in tiflash.xml

* Use `IManageableStorage` as interface for Storages synced from TiDB
* TMTStorages now store ptr to IManageableStore instead of StorageMergeTree

* Add `StorageDeltaMerge::deleteRange`
* Support `applySnapshot` by using `StorageDeltaMerge::deleteRange`

* Use ::DB::MutableSupport for constant naming in DeltaMergeDefines.h

* Note that we can NOT read data in KVStore by now, we must flush data to StorageDeltaMerge by using some commands like `DBGInvoke try_flush_region`
JaySon-Huang added a commit to JaySon-Huang/tiflash that referenced this pull request Oct 18, 2019
…ft (pingcap#217)

* Add enum StorageEngine for indicating which storage engine to use
* `DBGInvoke mock_tidb_table` now support Engine=DeltaMerge
* We can specify default storage engine by raft.storage_engine in tiflash.xml

* Use `IManageableStorage` as interface for Storages synced from TiDB
* TMTStorages now store ptr to IManageableStore instead of StorageMergeTree

* Add `StorageDeltaMerge::deleteRange`
* Support `applySnapshot` by using `StorageDeltaMerge::deleteRange`

* Use ::DB::MutableSupport for constant naming in DeltaMergeDefines.h

* Note that we can NOT read data in KVStore by now, we must flush data to StorageDeltaMerge by using some commands like `DBGInvoke try_flush_region`
JaySon-Huang added a commit that referenced this pull request Oct 22, 2019
…ft (#217)

* Add enum StorageEngine for indicating which storage engine to use
* `DBGInvoke mock_tidb_table` now support Engine=DeltaMerge
* We can specify default storage engine by raft.storage_engine in tiflash.xml

* Use `IManageableStorage` as interface for Storages synced from TiDB
* TMTStorages now store ptr to IManageableStore instead of StorageMergeTree

* Add `StorageDeltaMerge::deleteRange`
* Support `applySnapshot` by using `StorageDeltaMerge::deleteRange`

* Use ::DB::MutableSupport for constant naming in DeltaMergeDefines.h

* Note that we can NOT read data in KVStore by now, we must flush data to StorageDeltaMerge by using some commands like `DBGInvoke try_flush_region`
JaySon-Huang added a commit to JaySon-Huang/tiflash that referenced this pull request Oct 23, 2019
…ft (pingcap#217)

* Add enum StorageEngine for indicating which storage engine to use
* `DBGInvoke mock_tidb_table` now support Engine=DeltaMerge
* We can specify default storage engine by raft.storage_engine in tiflash.xml

* Use `IManageableStorage` as interface for Storages synced from TiDB
* TMTStorages now store ptr to IManageableStore instead of StorageMergeTree

* Add `StorageDeltaMerge::deleteRange`
* Support `applySnapshot` by using `StorageDeltaMerge::deleteRange`

* Use ::DB::MutableSupport for constant naming in DeltaMergeDefines.h

* Note that we can NOT read data in KVStore by now, we must flush data to StorageDeltaMerge by using some commands like `DBGInvoke try_flush_region`
JaySon-Huang added a commit to JaySon-Huang/tiflash that referenced this pull request Oct 30, 2019
…ft (pingcap#217)

* Add enum StorageEngine for indicating which storage engine to use
* `DBGInvoke mock_tidb_table` now support Engine=DeltaMerge
* We can specify default storage engine by raft.storage_engine in tiflash.xml

* Use `IManageableStorage` as interface for Storages synced from TiDB
* TMTStorages now store ptr to IManageableStore instead of StorageMergeTree

* Add `StorageDeltaMerge::deleteRange`
* Support `applySnapshot` by using `StorageDeltaMerge::deleteRange`

* Use ::DB::MutableSupport for constant naming in DeltaMergeDefines.h

* Note that we can NOT read data in KVStore by now, we must flush data to StorageDeltaMerge by using some commands like `DBGInvoke try_flush_region`
JaySon-Huang added a commit to JaySon-Huang/tiflash that referenced this pull request Nov 1, 2019
…ft (pingcap#217)

* Add enum StorageEngine for indicating which storage engine to use
* `DBGInvoke mock_tidb_table` now support Engine=DeltaMerge
* We can specify default storage engine by raft.storage_engine in tiflash.xml

* Use `IManageableStorage` as interface for Storages synced from TiDB
* TMTStorages now store ptr to IManageableStore instead of StorageMergeTree

* Add `StorageDeltaMerge::deleteRange`
* Support `applySnapshot` by using `StorageDeltaMerge::deleteRange`

* Use ::DB::MutableSupport for constant naming in DeltaMergeDefines.h

* Note that we can NOT read data in KVStore by now, we must flush data to StorageDeltaMerge by using some commands like `DBGInvoke try_flush_region`
JaySon-Huang added a commit to JaySon-Huang/tiflash that referenced this pull request Nov 1, 2019
…ft (pingcap#217)

* Add enum StorageEngine for indicating which storage engine to use
* `DBGInvoke mock_tidb_table` now support Engine=DeltaMerge
* We can specify default storage engine by raft.storage_engine in tiflash.xml

* Use `IManageableStorage` as interface for Storages synced from TiDB
* TMTStorages now store ptr to IManageableStore instead of StorageMergeTree

* Add `StorageDeltaMerge::deleteRange`
* Support `applySnapshot` by using `StorageDeltaMerge::deleteRange`

* Use ::DB::MutableSupport for constant naming in DeltaMergeDefines.h

* Note that we can NOT read data in KVStore by now, we must flush data to StorageDeltaMerge by using some commands like `DBGInvoke try_flush_region`
JaySon-Huang added a commit to JaySon-Huang/tiflash that referenced this pull request Nov 5, 2019
…ft (pingcap#217)

* Add enum StorageEngine for indicating which storage engine to use
* `DBGInvoke mock_tidb_table` now support Engine=DeltaMerge
* We can specify default storage engine by raft.storage_engine in tiflash.xml

* Use `IManageableStorage` as interface for Storages synced from TiDB
* TMTStorages now store ptr to IManageableStore instead of StorageMergeTree

* Add `StorageDeltaMerge::deleteRange`
* Support `applySnapshot` by using `StorageDeltaMerge::deleteRange`

* Use ::DB::MutableSupport for constant naming in DeltaMergeDefines.h

* Note that we can NOT read data in KVStore by now, we must flush data to StorageDeltaMerge by using some commands like `DBGInvoke try_flush_region`
JaySon-Huang pushed a commit to JaySon-Huang/tiflash that referenced this pull request Aug 2, 2024
Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants