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/04/16 06:46:15 UTC
[GitHub] [incubator-doris] caiconghui opened a new pull request #3330: Fix
rowset_meta race condition for commit_txn in TxnManager
caiconghui opened a new pull request #3330: Fix rowset_meta race condition for commit_txn in TxnManager
URL: https://github.com/apache/incubator-doris/pull/3330
This PR is to fix rowset_meta race condition for rowset_meta modification in commit_txn
----------------------------------------------------------------
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] kangkaisen commented on issue #3330: Fix
rowset_meta race condition for commit_txn in TxnManager
Posted by GitBox <gi...@apache.org>.
kangkaisen commented on issue #3330: Fix rowset_meta race condition for commit_txn in TxnManager
URL: https://github.com/apache/incubator-doris/pull/3330#issuecomment-614728935
>txn_lock to protect txn itself
what's the meaning of `protect txn itself` ?
> Only `commit_txn` and `publish_txn` has rowset meta modification. So I think the txn lock is mainly to protect the rowset meta of the specified txn.
@caiconghui Hi. If @morningman is right. Please rename `_txn_mutex` and `_txn_shard_size`, current name is confusing.
----------------------------------------------------------------
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] chaoyli edited a comment on issue #3330: Fix
rowset_meta race condition for commit_txn in TxnManager
Posted by GitBox <gi...@apache.org>.
chaoyli edited a comment on issue #3330: Fix rowset_meta race condition for commit_txn in TxnManager
URL: https://github.com/apache/incubator-doris/pull/3330#issuecomment-615031763
I think TxnManager is better to not know the RowsetMeta concept.
It's only to know the Transaction concept, so I think the previous name is OK.
----------------------------------------------------------------
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] chaoyli commented on issue #3330: Fix rowset_meta
race condition for commit_txn in TxnManager
Posted by GitBox <gi...@apache.org>.
chaoyli commented on issue #3330: Fix rowset_meta race condition for commit_txn in TxnManager
URL: https://github.com/apache/incubator-doris/pull/3330#issuecomment-615834669
You can use the _txn_mutex still
----------------------------------------------------------------
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 edited a comment on issue #3330: Fix
rowset_meta race condition for commit_txn in TxnManager
Posted by GitBox <gi...@apache.org>.
caiconghui edited a comment on issue #3330: Fix rowset_meta race condition for commit_txn in TxnManager
URL: https://github.com/apache/incubator-doris/pull/3330#issuecomment-615076301
> > I think TxnManager is better to not know the RowsetMeta concept.
> > It's only to know the Transaction concept, so I think the previous name is OK.
>
> 1. TxnManager **has known** the RowsetMeta concept.
> 2. `_txn_mutex` normally means we should get this lock for most of txn operations, but we only get it when `commit_txn` and `publish_txn`
> 3. We really use `_txn_mutex` to protect the rowset meta.
I think the txn_lock really ensure the commit_txn and publish_txn be 'atomic' function which cannot be interrupted by other thread, which not only include rowset_meta modification, function also need to find, insert or find and then delete the txn from txn_map.
@kangkaisen @morningman @chaoyli
----------------------------------------------------------------
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 #3330: Fix
rowset_meta race condition for commit_txn in TxnManager
Posted by GitBox <gi...@apache.org>.
caiconghui commented on issue #3330: Fix rowset_meta race condition for commit_txn in TxnManager
URL: https://github.com/apache/incubator-doris/pull/3330#issuecomment-614448085
for #3329
----------------------------------------------------------------
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] kangkaisen merged pull request #3330: Fix
rowset_meta race condition for commit_txn in TxnManager
Posted by GitBox <gi...@apache.org>.
kangkaisen merged pull request #3330: Fix rowset_meta race condition for commit_txn in TxnManager
URL: https://github.com/apache/incubator-doris/pull/3330
----------------------------------------------------------------
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 #3330: Fix
rowset_meta race condition for commit_txn in TxnManager
Posted by GitBox <gi...@apache.org>.
caiconghui commented on issue #3330: Fix rowset_meta race condition for commit_txn in TxnManager
URL: https://github.com/apache/incubator-doris/pull/3330#issuecomment-614706961
> @caiconghui Hi, Why `prepare_txn`, `rollback_txn`, `delete_txn` don't need this txn_lock?
@kangkaisen txn_lock to protect txn itself, and txn_map_lock to protect txn_map
----------------------------------------------------------------
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 #3330: Fix
rowset_meta race condition for commit_txn in TxnManager
Posted by GitBox <gi...@apache.org>.
caiconghui commented on issue #3330: Fix rowset_meta race condition for commit_txn in TxnManager
URL: https://github.com/apache/incubator-doris/pull/3330#issuecomment-614764900
> > txn_lock to protect txn itself
>
> what's the meaning of `protect txn itself` ?
>
> > Only `commit_txn` and `publish_txn` has rowset meta modification. So I think the txn lock is mainly to protect the rowset meta of the specified txn.
>
> @caiconghui Hi. If @morningman is right. Please rename `_txn_mutex` and `_txn_shard_size`, current name is confusing.
how about _rowset_meta_mutex, _rowset_meta_shard_size? @kangkaisen
----------------------------------------------------------------
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 issue #3330: Fix
rowset_meta race condition for commit_txn in TxnManager
Posted by GitBox <gi...@apache.org>.
morningman commented on issue #3330: Fix rowset_meta race condition for commit_txn in TxnManager
URL: https://github.com/apache/incubator-doris/pull/3330#issuecomment-614723544
> @caiconghui Hi, Why `prepare_txn`, `rollback_txn`, `delete_txn` don't need this txn_lock?
Only `commit_txn` and `publish_txn` has rowset meta modification. So I think the txn lock is mainly to protect the rowset meta of the specified txn.
----------------------------------------------------------------
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] chaoyli commented on issue #3330: Fix rowset_meta
race condition for commit_txn in TxnManager
Posted by GitBox <gi...@apache.org>.
chaoyli commented on issue #3330: Fix rowset_meta race condition for commit_txn in TxnManager
URL: https://github.com/apache/incubator-doris/pull/3330#issuecomment-615031763
I think TxnManager is better to not know the RowsetMeta concept.
----------------------------------------------------------------
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] kangkaisen commented on issue #3330: Fix
rowset_meta race condition for commit_txn in TxnManager
Posted by GitBox <gi...@apache.org>.
kangkaisen commented on issue #3330: Fix rowset_meta race condition for commit_txn in TxnManager
URL: https://github.com/apache/incubator-doris/pull/3330#issuecomment-614682795
@caiconghui Hi, Why `prepare_txn`, `rollback_txn`, `delete_txn` don't need this txn_lock?
----------------------------------------------------------------
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] chaoyli edited a comment on issue #3330: Fix
rowset_meta race condition for commit_txn in TxnManager
Posted by GitBox <gi...@apache.org>.
chaoyli edited a comment on issue #3330: Fix rowset_meta race condition for commit_txn in TxnManager
URL: https://github.com/apache/incubator-doris/pull/3330#issuecomment-615031763
I think TxnManager is better to not know the RowsetMeta concept.
It's only to know the Transaction concept.
----------------------------------------------------------------
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 #3330: Fix
rowset_meta race condition for commit_txn in TxnManager
Posted by GitBox <gi...@apache.org>.
caiconghui commented on issue #3330: Fix rowset_meta race condition for commit_txn in TxnManager
URL: https://github.com/apache/incubator-doris/pull/3330#issuecomment-615076301
> > I think TxnManager is better to not know the RowsetMeta concept.
> > It's only to know the Transaction concept, so I think the previous name is OK.
>
> 1. TxnManager **has known** the RowsetMeta concept.
> 2. `_txn_mutex` normally means we should get this lock for most of txn operations, but we only get it when `commit_txn` and `publish_txn`
> 3. We really use `_txn_mutex` to protect the rowset meta.
I think the txn_lock really ensure the commit_txn and publish_txn be atomic function, which not only include rowset_meta modification, function also need to find, insert or find and then delete the txn from txn_map.
@kangkaisen @morningman @chaoyli
----------------------------------------------------------------
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 edited a comment on issue #3330: Fix
rowset_meta race condition for commit_txn in TxnManager
Posted by GitBox <gi...@apache.org>.
caiconghui edited a comment on issue #3330: Fix rowset_meta race condition for commit_txn in TxnManager
URL: https://github.com/apache/incubator-doris/pull/3330#issuecomment-615076301
> > I think TxnManager is better to not know the RowsetMeta concept.
> > It's only to know the Transaction concept, so I think the previous name is OK.
>
> 1. TxnManager **has known** the RowsetMeta concept.
> 2. `_txn_mutex` normally means we should get this lock for most of txn operations, but we only get it when `commit_txn` and `publish_txn`
> 3. We really use `_txn_mutex` to protect the rowset meta.
I think the txn_lock really ensure the commit_txn and publish_txn be complete function which cannot be interrupted by other thread, which not only include rowset_meta modification, function also need to find, insert or find and then delete the txn from txn_map.
@kangkaisen @morningman @chaoyli
----------------------------------------------------------------
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] chaoyli edited a comment on issue #3330: Fix
rowset_meta race condition for commit_txn in TxnManager
Posted by GitBox <gi...@apache.org>.
chaoyli edited a comment on issue #3330: Fix rowset_meta race condition for commit_txn in TxnManager
URL: https://github.com/apache/incubator-doris/pull/3330#issuecomment-615834669
I discuss with @kangkaisen, you can use the _txn_mutex still.
----------------------------------------------------------------
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] kangkaisen commented on issue #3330: Fix
rowset_meta race condition for commit_txn in TxnManager
Posted by GitBox <gi...@apache.org>.
kangkaisen commented on issue #3330: Fix rowset_meta race condition for commit_txn in TxnManager
URL: https://github.com/apache/incubator-doris/pull/3330#issuecomment-615059049
> I think TxnManager is better to not know the RowsetMeta concept.
> It's only to know the Transaction concept, so I think the previous name is OK.
1. TxnManager **has known** the RowsetMeta concept.
2. `_txn_mutex` normally means we should get this lock for most of txn operations, but we only get it when `commit_txn` and `publish_txn`
3. We really use `_txn_mutex` to protect the rowset meta.
----------------------------------------------------------------
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] kangkaisen commented on issue #3330: Fix
rowset_meta race condition for commit_txn in TxnManager
Posted by GitBox <gi...@apache.org>.
kangkaisen commented on issue #3330: Fix rowset_meta race condition for commit_txn in TxnManager
URL: https://github.com/apache/incubator-doris/pull/3330#issuecomment-614987587
> how about _rowset_meta_mutex, _rowset_meta_shard_size? @kangkaisen
@caiconghui OK. I agree with you.
----------------------------------------------------------------
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