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/01/07 11:51:38 UTC
[GitHub] [incubator-doris] kangpinghuang opened a new pull request #2691:
add rowset state
kangpinghuang opened a new pull request #2691: add rowset state
URL: https://github.com/apache/incubator-doris/pull/2691
1. add rowset state to rowset
2. add close api to rowset to release resources
issue: #2665
----------------------------------------------------------------
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 #2691:
add rowset state
Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #2691: add rowset state
URL: https://github.com/apache/incubator-doris/pull/2691#discussion_r364267503
##########
File path: be/src/olap/rowset/rowset.cpp
##########
@@ -36,7 +37,34 @@ Rowset::Rowset(const TabletSchema *schema,
}
OLAPStatus Rowset::load(bool use_cache) {
- return _load_once.call([this, use_cache] { return do_load_once(use_cache); });
+ // if the state is ROWSET_UNLOADING it means close() is called
+ // and the rowset is already loaded, and the resource is not closed yet.
+ if (_rowset_state_machine.rowset_state() != ROWSET_UNLOADED) {
+ return OLAP_SUCCESS;
+ }
+ std::string load_log = "";
+ {
+ MutexLock load_lock(&_load_lock);
+ if (_rowset_state_machine.rowset_state() != ROWSET_UNLOADED) {
+ return OLAP_SUCCESS;
+ }
+ RETURN_NOT_OK(_rowset_state_machine.on_load());
+ RETURN_NOT_OK(do_load(use_cache));
+ std::stringstream ss;
+ ss << "rowset is loaded. rowset version:" << start_version() << "-" << end_version()
+ << ", state from ROWSET_UNLOADED to ROWSET_LOADED. tabletid:"
+ << _rowset_meta->tablet_id();
+ load_log = ss.str();
+ }
+ if (load_log != "") {
+ LOG(INFO) << load_log;
+ }
+ return OLAP_SUCCESS;
+}
+
+OLAPStatus Rowset::create_reader(std::shared_ptr<RowsetReader>* result) {
+ std::lock_guard<SpinLock> l(_lock);
Review comment:
Is this lock needed?
----------------------------------------------------------------
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 #2691: add rowset state
Posted by GitBox <gi...@apache.org>.
imay merged pull request #2691: add rowset state
URL: https://github.com/apache/incubator-doris/pull/2691
----------------------------------------------------------------
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] kangpinghuang commented on a change in pull
request #2691: add rowset state
Posted by GitBox <gi...@apache.org>.
kangpinghuang commented on a change in pull request #2691: add rowset state
URL: https://github.com/apache/incubator-doris/pull/2691#discussion_r364545507
##########
File path: be/src/olap/rowset/rowset.cpp
##########
@@ -36,7 +37,34 @@ Rowset::Rowset(const TabletSchema *schema,
}
OLAPStatus Rowset::load(bool use_cache) {
- return _load_once.call([this, use_cache] { return do_load_once(use_cache); });
+ // if the state is ROWSET_UNLOADING it means close() is called
+ // and the rowset is already loaded, and the resource is not closed yet.
+ if (_rowset_state_machine.rowset_state() != ROWSET_UNLOADED) {
+ return OLAP_SUCCESS;
+ }
+ std::string load_log = "";
+ {
+ MutexLock load_lock(&_load_lock);
+ if (_rowset_state_machine.rowset_state() != ROWSET_UNLOADED) {
+ return OLAP_SUCCESS;
+ }
+ RETURN_NOT_OK(_rowset_state_machine.on_load());
+ RETURN_NOT_OK(do_load(use_cache));
+ std::stringstream ss;
+ ss << "rowset is loaded. rowset version:" << start_version() << "-" << end_version()
+ << ", state from ROWSET_UNLOADED to ROWSET_LOADED. tabletid:"
+ << _rowset_meta->tablet_id();
+ load_log = ss.str();
+ }
+ if (load_log != "") {
+ LOG(INFO) << load_log;
+ }
+ return OLAP_SUCCESS;
+}
+
+OLAPStatus Rowset::create_reader(std::shared_ptr<RowsetReader>* result) {
+ std::lock_guard<SpinLock> l(_lock);
Review comment:
useless
----------------------------------------------------------------
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 #2691:
add rowset state
Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #2691: add rowset state
URL: https://github.com/apache/incubator-doris/pull/2691#discussion_r364265877
##########
File path: be/src/olap/rowset/rowset.h
##########
@@ -95,6 +151,37 @@ class Rowset : public std::enable_shared_from_this<Rowset> {
// TODO should we rename the method to remove_files() to be more specific?
virtual OLAPStatus remove() = 0;
+ // close to clear the resource owned by rowset
+ // including: open files, indexes and so on
+ // NOTICE: can not call this function in multithreads
+ void close() {
+ RowsetState old_state = _rowset_state_machine.rowset_state();
+ if (old_state == ROWSET_UNLOADED) {
Review comment:
When the state is unloading, seems OK to return
----------------------------------------------------------------
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 #2691:
add rowset state
Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #2691: add rowset state
URL: https://github.com/apache/incubator-doris/pull/2691#discussion_r364264929
##########
File path: be/src/olap/rowset/rowset.h
##########
@@ -127,6 +214,30 @@ class Rowset : public std::enable_shared_from_this<Rowset> {
return left->end_version() < right->end_version();
}
+ // this function is called by reader to increase reference of rowset
+ void aquire() {
+ ++_refs_by_reader;
+ }
+
+ void release() {
+ // if the refs by reader is 0 and the rowset is closed, should release the resouce
+ uint64_t current_refs = --_refs_by_reader;
+ if (current_refs == 0 && _rowset_state_machine.rowset_state() == ROWSET_UNLOADING) {
+ auto st = _rowset_state_machine.on_close(current_refs);
+ if (st != OLAP_SUCCESS) {
Review comment:
seems no usage, just leave a DCHECK(state == UNLOADED)
----------------------------------------------------------------
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] kangpinghuang commented on a change in pull
request #2691: add rowset state
Posted by GitBox <gi...@apache.org>.
kangpinghuang commented on a change in pull request #2691: add rowset state
URL: https://github.com/apache/incubator-doris/pull/2691#discussion_r364545465
##########
File path: be/src/olap/rowset/rowset.h
##########
@@ -95,6 +151,37 @@ class Rowset : public std::enable_shared_from_this<Rowset> {
// TODO should we rename the method to remove_files() to be more specific?
virtual OLAPStatus remove() = 0;
+ // close to clear the resource owned by rowset
+ // including: open files, indexes and so on
+ // NOTICE: can not call this function in multithreads
+ void close() {
+ RowsetState old_state = _rowset_state_machine.rowset_state();
+ if (old_state == ROWSET_UNLOADED) {
Review comment:
yes, u are right.
----------------------------------------------------------------
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 #2691:
add rowset state
Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #2691: add rowset state
URL: https://github.com/apache/incubator-doris/pull/2691#discussion_r364262487
##########
File path: be/src/olap/rowset/rowset.h
##########
@@ -152,8 +268,13 @@ class Rowset : public std::enable_shared_from_this<Rowset> {
bool _is_pending; // rowset is pending iff it's not in visible state
bool _is_cumulative; // rowset is cumulative iff it's visible and start version < end version
- DorisCallOnce<OLAPStatus> _load_once;
+ SpinLock _lock;
+ Mutex _load_lock;
bool _need_delete_file = false;
+ // variable to indicate how many rowset readers owned this rowset
+ std::atomic<uint64_t> _refs_by_reader;
+ // rowset state machine
+ RowsetStateMachine _rowset_state_machine;
Review comment:
should be given a default value
----------------------------------------------------------------
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] kangpinghuang opened a new pull request #2691:
add rowset state
Posted by GitBox <gi...@apache.org>.
kangpinghuang opened a new pull request #2691: add rowset state
URL: https://github.com/apache/incubator-doris/pull/2691
1. add rowset state to rowset
2. add close api to rowset to release resources
issue: #2665
----------------------------------------------------------------
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] kangpinghuang commented on a change in pull
request #2691: add rowset state
Posted by GitBox <gi...@apache.org>.
kangpinghuang commented on a change in pull request #2691: add rowset state
URL: https://github.com/apache/incubator-doris/pull/2691#discussion_r364680612
##########
File path: be/src/olap/rowset/alpha_rowset.cpp
##########
@@ -51,7 +51,8 @@ OLAPStatus AlphaRowset::do_load(bool use_cache) {
return OLAP_SUCCESS;
}
-OLAPStatus AlphaRowset::do_create_reader(std::shared_ptr<RowsetReader>* result) {
+OLAPStatus AlphaRowset::create_reader(std::shared_ptr<RowsetReader>* result) {
+ MutexLock release_lock(&_lock);
Review comment:
yes, there should be no lock here, I will remove it.
load is called in init()
----------------------------------------------------------------
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] kangpinghuang closed pull request #2691: add
rowset state
Posted by GitBox <gi...@apache.org>.
kangpinghuang closed pull request #2691: add rowset state
URL: https://github.com/apache/incubator-doris/pull/2691
----------------------------------------------------------------
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 #2691:
add rowset state
Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #2691: add rowset state
URL: https://github.com/apache/incubator-doris/pull/2691#discussion_r364675628
##########
File path: be/src/olap/rowset/alpha_rowset.cpp
##########
@@ -51,7 +51,8 @@ OLAPStatus AlphaRowset::do_load(bool use_cache) {
return OLAP_SUCCESS;
}
-OLAPStatus AlphaRowset::do_create_reader(std::shared_ptr<RowsetReader>* result) {
+OLAPStatus AlphaRowset::create_reader(std::shared_ptr<RowsetReader>* result) {
+ MutexLock release_lock(&_lock);
Review comment:
why lock? I think you should call load to make sure rowset is loaded?
----------------------------------------------------------------
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] kangpinghuang closed pull request #2691: add
rowset state
Posted by GitBox <gi...@apache.org>.
kangpinghuang closed pull request #2691: add rowset state
URL: https://github.com/apache/incubator-doris/pull/2691
----------------------------------------------------------------
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] kangpinghuang closed pull request #2691: add
rowset state
Posted by GitBox <gi...@apache.org>.
kangpinghuang closed pull request #2691: add rowset state
URL: https://github.com/apache/incubator-doris/pull/2691
----------------------------------------------------------------
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 #2691:
add rowset state
Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #2691: add rowset state
URL: https://github.com/apache/incubator-doris/pull/2691#discussion_r364264194
##########
File path: be/src/olap/rowset/rowset.h
##########
@@ -95,6 +151,37 @@ class Rowset : public std::enable_shared_from_this<Rowset> {
// TODO should we rename the method to remove_files() to be more specific?
virtual OLAPStatus remove() = 0;
+ // close to clear the resource owned by rowset
+ // including: open files, indexes and so on
+ // NOTICE: can not call this function in multithreads
+ void close() {
+ RowsetState old_state = _rowset_state_machine.rowset_state();
+ if (old_state == ROWSET_UNLOADED) {
+ return;
+ }
+ OLAPStatus st = OLAP_SUCCESS;
+ {
+ std::lock_guard<SpinLock> l(_lock);
+ old_state = _rowset_state_machine.rowset_state();
+ if (old_state == ROWSET_UNLOADED) {
+ return;
+ }
+ uint64_t current_refs = _refs_by_reader;
+ st = _rowset_state_machine.on_close(current_refs);
+ if (current_refs == 0) {
+ do_close();
Review comment:
better to call this out of lock block
----------------------------------------------------------------
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] kangpinghuang opened a new pull request #2691:
add rowset state
Posted by GitBox <gi...@apache.org>.
kangpinghuang opened a new pull request #2691: add rowset state
URL: https://github.com/apache/incubator-doris/pull/2691
1. add rowset state to rowset
2. add close api to rowset to release resources
issue: #2665
----------------------------------------------------------------
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] kangpinghuang opened a new pull request #2691:
add rowset state
Posted by GitBox <gi...@apache.org>.
kangpinghuang opened a new pull request #2691: add rowset state
URL: https://github.com/apache/incubator-doris/pull/2691
1. add rowset state to rowset
2. add close api to rowset to release resources
issue: #2665
----------------------------------------------------------------
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] kangpinghuang commented on a change in pull
request #2691: add rowset state
Posted by GitBox <gi...@apache.org>.
kangpinghuang commented on a change in pull request #2691: add rowset state
URL: https://github.com/apache/incubator-doris/pull/2691#discussion_r364543721
##########
File path: be/src/olap/rowset/rowset.h
##########
@@ -152,8 +268,13 @@ class Rowset : public std::enable_shared_from_this<Rowset> {
bool _is_pending; // rowset is pending iff it's not in visible state
bool _is_cumulative; // rowset is cumulative iff it's visible and start version < end version
- DorisCallOnce<OLAPStatus> _load_once;
+ SpinLock _lock;
+ Mutex _load_lock;
Review comment:
done
----------------------------------------------------------------
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 #2691:
add rowset state
Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #2691: add rowset state
URL: https://github.com/apache/incubator-doris/pull/2691#discussion_r364266255
##########
File path: be/src/olap/rowset/rowset.h
##########
@@ -95,6 +151,37 @@ class Rowset : public std::enable_shared_from_this<Rowset> {
// TODO should we rename the method to remove_files() to be more specific?
virtual OLAPStatus remove() = 0;
+ // close to clear the resource owned by rowset
+ // including: open files, indexes and so on
+ // NOTICE: can not call this function in multithreads
+ void close() {
+ RowsetState old_state = _rowset_state_machine.rowset_state();
+ if (old_state == ROWSET_UNLOADED) {
+ return;
+ }
+ OLAPStatus st = OLAP_SUCCESS;
+ {
+ std::lock_guard<SpinLock> l(_lock);
+ old_state = _rowset_state_machine.rowset_state();
+ if (old_state == ROWSET_UNLOADED) {
Review comment:
UNLOADING
----------------------------------------------------------------
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 #2691:
add rowset state
Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #2691: add rowset state
URL: https://github.com/apache/incubator-doris/pull/2691#discussion_r364262187
##########
File path: be/src/olap/rowset/rowset.h
##########
@@ -152,8 +268,13 @@ class Rowset : public std::enable_shared_from_this<Rowset> {
bool _is_pending; // rowset is pending iff it's not in visible state
bool _is_cumulative; // rowset is cumulative iff it's visible and start version < end version
- DorisCallOnce<OLAPStatus> _load_once;
+ SpinLock _lock;
+ Mutex _load_lock;
Review comment:
give comment for the usage of these two 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