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/02/18 11:42:31 UTC
[GitHub] [incubator-doris] lingbin opened a new pull request #2933: Use
`std::bind` to replace `boost::bind` in flush-executor
lingbin opened a new pull request #2933: Use `std::bind` to replace `boost::bind` in flush-executor
URL: https://github.com/apache/incubator-doris/pull/2933
This patch also removes some unneeded headers.
this is an TODO in #2931
----------------------------------------------------------------
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] imay commented on a change in pull request #2933:
Fix the flush_status bug in flush-executor
Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #2933: Fix the flush_status bug in flush-executor
URL: https://github.com/apache/incubator-doris/pull/2933#discussion_r381026199
##########
File path: be/src/olap/memtable_flush_executor.cpp
##########
@@ -35,15 +31,15 @@ std::ostream& operator<<(std::ostream& os, const FlushStatistic& stat) {
}
OLAPStatus FlushToken::submit(std::shared_ptr<MemTable> memtable) {
Review comment:
`std::bind` below copies `memtable`, which will keep it valid in your mentioned situation. So I think it is OK to make the function's argument reference.
----------------------------------------------------------------
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] lingbin commented on a change in pull request
#2933: Fix the flush_status bug in flush-executor
Posted by GitBox <gi...@apache.org>.
lingbin commented on a change in pull request #2933: Fix the flush_status bug in flush-executor
URL: https://github.com/apache/incubator-doris/pull/2933#discussion_r381164233
##########
File path: be/src/olap/memtable_flush_executor.cpp
##########
@@ -35,15 +31,15 @@ std::ostream& operator<<(std::ostream& os, const FlushStatistic& stat) {
}
OLAPStatus FlushToken::submit(std::shared_ptr<MemTable> memtable) {
Review comment:
Oh, I get you. I will change the param to a reference.
----------------------------------------------------------------
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] lingbin commented on a change in pull request
#2933: Use `std::bind` to replace `boost::bind` in flush-executor
Posted by GitBox <gi...@apache.org>.
lingbin commented on a change in pull request #2933: Use `std::bind` to replace `boost::bind` in flush-executor
URL: https://github.com/apache/incubator-doris/pull/2933#discussion_r380753659
##########
File path: be/src/olap/memtable_flush_executor.cpp
##########
@@ -35,15 +31,15 @@ std::ostream& operator<<(std::ostream& os, const FlushStatistic& stat) {
}
OLAPStatus FlushToken::submit(std::shared_ptr<MemTable> memtable) {
Review comment:
After the `submit()` returns, the upper layer will immediately reset the `memtable` (let it be `nullptr`), and the lower thread pool may not yet start processing it. If a reference is used, when the thread pool starts to process this `memtable`, the corresponding object may have become invalid.
----------------------------------------------------------------
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] imay merged pull request #2933: Fix the
flush_status bug in flush-executor
Posted by GitBox <gi...@apache.org>.
imay merged pull request #2933: Fix the flush_status bug in flush-executor
URL: https://github.com/apache/incubator-doris/pull/2933
----------------------------------------------------------------
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] imay commented on a change in pull request #2933:
Use `std::bind` to replace `boost::bind` in flush-executor
Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #2933: Use `std::bind` to replace `boost::bind` in flush-executor
URL: https://github.com/apache/incubator-doris/pull/2933#discussion_r380666858
##########
File path: be/src/olap/memtable_flush_executor.cpp
##########
@@ -35,15 +31,15 @@ std::ostream& operator<<(std::ostream& os, const FlushStatistic& stat) {
}
OLAPStatus FlushToken::submit(std::shared_ptr<MemTable> memtable) {
Review comment:
why not use `const std::shared_ptr<MemTable>&` to avoid a copy assignment?
----------------------------------------------------------------
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