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/07 08:14:19 UTC
[GitHub] [incubator-doris] gaodayue opened a new pull request #3271: [Bug]
Avoid compacting recently added rowset
gaodayue opened a new pull request #3271: [Bug] Avoid compacting recently added rowset
URL: https://github.com/apache/incubator-doris/pull/3271
This CL fixes #3270 by skipping recently added version when performing cumulative compaction. A new config named "cumulative_compaction_skip_window_seconds" is added to adjust the time window.
----------------------------------------------------------------
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] gaodayue commented on a change in pull request
#3271: [Bug] Avoid compacting recently added rowset
Posted by GitBox <gi...@apache.org>.
gaodayue commented on a change in pull request #3271: [Bug] Avoid compacting recently added rowset
URL: https://github.com/apache/incubator-doris/pull/3271#discussion_r405241070
##########
File path: be/src/olap/rowset/rowset.cpp
##########
@@ -65,6 +67,9 @@ void Rowset::make_visible(Version version, VersionHash version_hash) {
_rowset_meta->set_version(version);
_rowset_meta->set_version_hash(version_hash);
_rowset_meta->set_rowset_state(VISIBLE);
+ // update create time to the visible time,
+ // it's used to skip recently published version during compaction
+ _rowset_meta->set_creation_time(time(nullptr));
Review comment:
OK, I'll use functions in util/time.h instead. However I don't think monotonic time is appropriate here for two reasons
1. Previous rowsets use unix timestamp as `creation_time`, switch to monotonic time will break other parts of code that uses that field
2. Monotonic time may go backward after system reboot, which is not suitable for persistence
----------------------------------------------------------------
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 issue #3271: [Bug] Avoid
compacting recently added rowset
Posted by GitBox <gi...@apache.org>.
imay commented on issue #3271: [Bug] Avoid compacting recently added rowset
URL: https://github.com/apache/incubator-doris/pull/3271#issuecomment-610729740
Hi, @gaodayue
After #3264 is merged, backend's configure can be changed without restart. Better to make cumulative_compaction_skip_window_seconds modifiable.
----------------------------------------------------------------
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] gaodayue commented on a change in pull request
#3271: [Bug] Avoid compacting recently added rowset
Posted by GitBox <gi...@apache.org>.
gaodayue commented on a change in pull request #3271: [Bug] Avoid compacting recently added rowset
URL: https://github.com/apache/incubator-doris/pull/3271#discussion_r404778443
##########
File path: be/src/olap/rowset/rowset.cpp
##########
@@ -65,6 +67,9 @@ void Rowset::make_visible(Version version, VersionHash version_hash) {
_rowset_meta->set_version(version);
_rowset_meta->set_version_hash(version_hash);
_rowset_meta->set_rowset_state(VISIBLE);
+ // update create time to the visible time,
+ // it's used to skip recently published version during compaction
+ _rowset_meta->set_creation_time(time(nullptr));
Review comment:
You mean `UnixMillis() / 1000`? Any reason for that, it seems more verbose than time(nullptr).
----------------------------------------------------------------
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] gaodayue opened a new pull request #3271: [Bug]
Avoid compacting recently added rowset
Posted by GitBox <gi...@apache.org>.
gaodayue opened a new pull request #3271: [Bug] Avoid compacting recently added rowset
URL: https://github.com/apache/incubator-doris/pull/3271
This CL fixes #3270 by skipping recently added version when performing cumulative compaction. A new config named "cumulative_compaction_skip_window_seconds" is added to adjust the time window.
----------------------------------------------------------------
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] gaodayue commented on issue #3271: [Bug] Avoid
compacting recently added rowset
Posted by GitBox <gi...@apache.org>.
gaodayue commented on issue #3271: [Bug] Avoid compacting recently added rowset
URL: https://github.com/apache/incubator-doris/pull/3271#issuecomment-610730176
@imay that's great, let me rebase
----------------------------------------------------------------
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] gaodayue closed pull request #3271: [Bug] Avoid
compacting recently added rowset
Posted by GitBox <gi...@apache.org>.
gaodayue closed pull request #3271: [Bug] Avoid compacting recently added rowset
URL: https://github.com/apache/incubator-doris/pull/3271
----------------------------------------------------------------
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 #3271:
[Bug] Avoid compacting recently added rowset
Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #3271: [Bug] Avoid compacting recently added rowset
URL: https://github.com/apache/incubator-doris/pull/3271#discussion_r405242465
##########
File path: be/src/olap/rowset/rowset.cpp
##########
@@ -65,6 +67,9 @@ void Rowset::make_visible(Version version, VersionHash version_hash) {
_rowset_meta->set_version(version);
_rowset_meta->set_version_hash(version_hash);
_rowset_meta->set_rowset_state(VISIBLE);
+ // update create time to the visible time,
+ // it's used to skip recently published version during compaction
+ _rowset_meta->set_creation_time(time(nullptr));
Review comment:
Yes, you're right. Last night when I remembered that create time will be saved, then I realize that the monotonic time is not suitable.
----------------------------------------------------------------
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 #3271:
[Bug] Avoid compacting recently added rowset
Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #3271: [Bug] Avoid compacting recently added rowset
URL: https://github.com/apache/incubator-doris/pull/3271#discussion_r404629225
##########
File path: be/src/olap/rowset/rowset.cpp
##########
@@ -65,6 +67,9 @@ void Rowset::make_visible(Version version, VersionHash version_hash) {
_rowset_meta->set_version(version);
_rowset_meta->set_version_hash(version_hash);
_rowset_meta->set_rowset_state(VISIBLE);
+ // update create time to the visible time,
+ // it's used to skip recently published version during compaction
+ _rowset_meta->set_creation_time(time(nullptr));
Review comment:
Better to use function in util/time.h.
----------------------------------------------------------------
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 issue #3271: [Bug] Avoid
compacting recently added rowset
Posted by GitBox <gi...@apache.org>.
imay commented on issue #3271: [Bug] Avoid compacting recently added rowset
URL: https://github.com/apache/incubator-doris/pull/3271#issuecomment-610891375
I have run this UT manually, and it is successful.
So, I will merge this PR.
----------------------------------------------------------------
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 #3271:
[Bug] Avoid compacting recently added rowset
Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #3271: [Bug] Avoid compacting recently added rowset
URL: https://github.com/apache/incubator-doris/pull/3271#discussion_r404785994
##########
File path: be/src/olap/rowset/rowset.cpp
##########
@@ -65,6 +67,9 @@ void Rowset::make_visible(Version version, VersionHash version_hash) {
_rowset_meta->set_version(version);
_rowset_meta->set_version_hash(version_hash);
_rowset_meta->set_rowset_state(VISIBLE);
+ // update create time to the visible time,
+ // it's used to skip recently published version during compaction
+ _rowset_meta->set_creation_time(time(nullptr));
Review comment:
Just to unify. I think it will be better to use functions in this file to get all kinds of current time.
Actually, it is OK for this case to get a monotonic time other than wall time, which will still work when system time rollback.
----------------------------------------------------------------
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 #3271: [Bug] Avoid
compacting recently added rowset
Posted by GitBox <gi...@apache.org>.
imay merged pull request #3271: [Bug] Avoid compacting recently added rowset
URL: https://github.com/apache/incubator-doris/pull/3271
----------------------------------------------------------------
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