You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2020/03/28 14:26:48 UTC
[GitHub] [incubator-doris] caiconghui opened a new pull request #3222:
Support sharding txn_map_lock into more small map locks to make good
performance for txn manage task
caiconghui opened a new pull request #3222: Support sharding txn_map_lock into more small map locks to make good performance for txn manage task
URL: https://github.com/apache/incubator-doris/pull/3222
This PR is to enhance the peformance for txn manage task, when there are so many txn in doris be, the only one txn_map_lock and additional _txn_locks may cause poor performance, and now we split it into many small locks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] caiconghui commented on issue #3222: Support
sharding txn_map_lock into more small map locks to make good performance
for txn manage task
Posted by GitBox <gi...@apache.org>.
caiconghui commented on issue #3222: Support sharding txn_map_lock into more small map locks to make good performance for txn manage task
URL: https://github.com/apache/incubator-doris/pull/3222#issuecomment-605455862
for #3223
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on a change in pull request
#3222: Support sharding txn_map_lock into more small map locks to make good
performance for txn manage task
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3222: Support sharding txn_map_lock into more small map locks to make good performance for txn manage task
URL: https://github.com/apache/incubator-doris/pull/3222#discussion_r400935246
##########
File path: be/src/olap/txn_manager.cpp
##########
@@ -70,10 +71,13 @@ using std::vector;
namespace doris {
-TxnManager::TxnManager() {
- for (int i = 0; i < _txn_lock_num; ++i) {
- _txn_locks[i] = std::make_shared<RWMutex>();
- }
+TxnManager::TxnManager(int32_t txn_map_shard_size)
+ : _txn_map_shard_size(txn_map_shard_size) {
+ DCHECK_GT(_txn_map_shard_size, 0);
+ DCHECK_EQ(_txn_map_shard_size & (_txn_map_shard_size - 1), 0);
+ _txn_map_locks = new RWMutex[_txn_map_shard_size];
+ _txn_tablet_maps = new std::map<TxnKey, std::map<TabletInfo, TabletTxnInfo>>[_txn_map_shard_size];
Review comment:
```suggestion
_txn_tablet_maps = new txn_tablet_map_t[_txn_map_shard_size];
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on a change in pull request
#3222: Support sharding txn_map_lock into more small map locks to make good
performance for txn manage task
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3222: Support sharding txn_map_lock into more small map locks to make good performance for txn manage task
URL: https://github.com/apache/incubator-doris/pull/3222#discussion_r400933966
##########
File path: be/src/olap/txn_manager.h
##########
@@ -140,30 +141,48 @@ class TxnManager {
void get_partition_ids(const TTransactionId transaction_id, std::vector<TPartitionId>* partition_ids);
private:
- RWMutex* _get_txn_lock(TTransactionId txn_id) {
- return _txn_locks[txn_id % _txn_lock_num].get();
- }
+
+ using TxnKey = std::pair<int64_t, int64_t>; // partition_id, transaction_id;
+
+ typedef std::map<TxnKey, std::map<TabletInfo, TabletTxnInfo>> txn_tablet_map_t;
+ typedef std::unordered_map<int64_t, std::unordered_set<int64_t>> txn_partition_map_t;
+
+ inline RWMutex& _get_txn_map_lock(TTransactionId transactionId);
+
+ inline txn_tablet_map_t& _get_txn_tablet_map(TTransactionId transactionId);
+
+ inline txn_partition_map_t& _get_txn_partition_map(TTransactionId transactionId);
// insert or remove (transaction_id, partition_id) from _txn_partition_map
// get _txn_map_lock before calling
void _insert_txn_partition_map_unlocked(int64_t transaction_id, int64_t partition_id);
void _clear_txn_partition_map_unlocked(int64_t transaction_id, int64_t partition_id);
private:
- RWMutex _txn_map_lock;
- using TxnKey = std::pair<int64_t, int64_t>; // partition_id, transaction_id;
- std::map<TxnKey, std::map<TabletInfo, TabletTxnInfo>> _txn_tablet_map;
+ const int32_t _txn_map_shard_size;
+
+ // _txn_map_locks[i] protect _txn_tablet_maps[i], i=0,1,2...,and i < _txn_map_shard_size
+ std::map<TxnKey, std::map<TabletInfo, TabletTxnInfo>> *_txn_tablet_maps;
Review comment:
And using `std::unordered_map` is better
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on a change in pull request
#3222: Support sharding txn_map_lock into more small map locks to make good
performance for txn manage task
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3222: Support sharding txn_map_lock into more small map locks to make good performance for txn manage task
URL: https://github.com/apache/incubator-doris/pull/3222#discussion_r400932172
##########
File path: be/src/olap/txn_manager.h
##########
@@ -140,30 +141,48 @@ class TxnManager {
void get_partition_ids(const TTransactionId transaction_id, std::vector<TPartitionId>* partition_ids);
private:
- RWMutex* _get_txn_lock(TTransactionId txn_id) {
- return _txn_locks[txn_id % _txn_lock_num].get();
- }
+
+ using TxnKey = std::pair<int64_t, int64_t>; // partition_id, transaction_id;
+
+ typedef std::map<TxnKey, std::map<TabletInfo, TabletTxnInfo>> txn_tablet_map_t;
+ typedef std::unordered_map<int64_t, std::unordered_set<int64_t>> txn_partition_map_t;
+
+ inline RWMutex& _get_txn_map_lock(TTransactionId transactionId);
+
+ inline txn_tablet_map_t& _get_txn_tablet_map(TTransactionId transactionId);
+
+ inline txn_partition_map_t& _get_txn_partition_map(TTransactionId transactionId);
// insert or remove (transaction_id, partition_id) from _txn_partition_map
// get _txn_map_lock before calling
void _insert_txn_partition_map_unlocked(int64_t transaction_id, int64_t partition_id);
void _clear_txn_partition_map_unlocked(int64_t transaction_id, int64_t partition_id);
private:
- RWMutex _txn_map_lock;
- using TxnKey = std::pair<int64_t, int64_t>; // partition_id, transaction_id;
- std::map<TxnKey, std::map<TabletInfo, TabletTxnInfo>> _txn_tablet_map;
+ const int32_t _txn_map_shard_size;
+
+ // _txn_map_locks[i] protect _txn_tablet_maps[i], i=0,1,2...,and i < _txn_map_shard_size
+ std::map<TxnKey, std::map<TabletInfo, TabletTxnInfo>> *_txn_tablet_maps;
Review comment:
```suggestion
txn_tablet_map_t *_txn_tablet_maps;
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman merged pull request #3222: Support
sharding txn_map_lock into more small map locks to make good performance
for txn manage task
Posted by GitBox <gi...@apache.org>.
morningman merged pull request #3222: Support sharding txn_map_lock into more small map locks to make good performance for txn manage task
URL: https://github.com/apache/incubator-doris/pull/3222
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on a change in pull request
#3222: Support sharding txn_map_lock into more small map locks to make good
performance for txn manage task
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3222: Support sharding txn_map_lock into more small map locks to make good performance for txn manage task
URL: https://github.com/apache/incubator-doris/pull/3222#discussion_r400945083
##########
File path: be/src/olap/txn_manager.cpp
##########
@@ -253,11 +260,11 @@ OLAPStatus TxnManager::publish_txn(OlapMeta* meta, TPartitionId partition_id, TT
pair<int64_t, int64_t> key(partition_id, transaction_id);
TabletInfo tablet_info(tablet_id, schema_hash, tablet_uid);
RowsetSharedPtr rowset_ptr = nullptr;
- WriteLock wrlock(_get_txn_lock(transaction_id));
Review comment:
I think you may not be able to remove this lock.
`WriteLock wrlock(_get_txn_lock(transaction_id));`
The meaning of this lock is different from that of the `_txn_map_lock`. This lock prevents a single transaction from being modified at the same time. The `_txn_map_lock` prevents the map structure from being modified at the same time.
If this lock is removed here. Then there may be two threads publishing the txn at the same time, which will modify their contents(such as rowset meta) at the same time.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org