You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "freemandealer (via GitHub)" <gi...@apache.org> on 2023/04/10 12:12:59 UTC
[GitHub] [doris] freemandealer opened a new pull request, #18528: [fix](storage) free dict_page immediately after usage to save memory
freemandealer opened a new pull request, #18528:
URL: https://github.com/apache/doris/pull/18528
The dictionary may be used for first few data pages but after a certain point, it will no longer be needed. Free it immediately to save memory.
# Proposed changes
Issue Number: close #xxx
## Problem summary
Describe your changes.
## Checklist(Required)
* [ ] Does it affect the original behavior
* [ ] Has unit tests been added
* [ ] Has document been added or modified
* [ ] Does it need to update dependencies
* [ ] Is this PR support rollback (If NO, please explain WHY)
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] freemandealer commented on a diff in pull request #18528: [fix](storage) free dict_page immediately after usage to save memory
Posted by "freemandealer (via GitHub)" <gi...@apache.org>.
freemandealer commented on code in PR #18528:
URL: https://github.com/apache/doris/pull/18528#discussion_r1162249772
##########
be/src/olap/rowset/segment_v2/column_reader.cpp:
##########
@@ -1107,9 +1107,10 @@ Status FileColumnIterator::_read_data_page(const OrdinalPageIndexIterator& iter)
Slice dict_data;
PageFooterPB dict_footer;
_opts.type = INDEX_PAGE;
Review Comment:
> If index is used, is there such case that we read page number 10 first and then read page number 1? Or in other words, could we make sure that the data page is read in descending order?
no sure, I will figure it out somehow
##########
be/src/olap/rowset/segment_v2/column_reader.cpp:
##########
@@ -1107,9 +1107,10 @@ Status FileColumnIterator::_read_data_page(const OrdinalPageIndexIterator& iter)
Slice dict_data;
PageFooterPB dict_footer;
_opts.type = INDEX_PAGE;
Review Comment:
> If index is used, is there such case that we read page number 10 first and then read page number 1? Or in other words, could we make sure that the data page is read in descending order?
not sure, I will figure it out somehow
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] freemandealer commented on pull request #18528: [wip][fix](storage) free dict_page immediately after usage to save memory
Posted by "freemandealer (via GitHub)" <gi...@apache.org>.
freemandealer commented on PR #18528:
URL: https://github.com/apache/doris/pull/18528#issuecomment-1628044100
not compatible with 'in list'
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] zhannngchen commented on a diff in pull request #18528: [fix](storage) free dict_page immediately after usage to save memory
Posted by "zhannngchen (via GitHub)" <gi...@apache.org>.
zhannngchen commented on code in PR #18528:
URL: https://github.com/apache/doris/pull/18528#discussion_r1161681716
##########
be/src/olap/rowset/segment_v2/column_reader.h:
##########
@@ -359,7 +359,7 @@ class FileColumnIterator final : public ColumnIterator {
std::unique_ptr<PageDecoder> _dict_decoder;
// keep dict page handle to avoid released
- PageHandle _dict_page_handle;
+ PageHandle* _dict_page_handle = nullptr;
Review Comment:
We'd better to avoid use pointer directly, consider unique_ptr to avoid potential memory leak?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #18528: [fix](storage) free dict_page immediately after usage to save memory
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18528:
URL: https://github.com/apache/doris/pull/18528#issuecomment-1504952701
clang-tidy review says "All clean, LGTM! :+1:"
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #18528: [fix](storage) free dict_page immediately after usage to save memory
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18528:
URL: https://github.com/apache/doris/pull/18528#issuecomment-1501942750
PR approved by anyone and no changes requested.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] freemandealer commented on pull request #18528: [fix](storage) free dict_page immediately after usage to save memory
Posted by "freemandealer (via GitHub)" <gi...@apache.org>.
freemandealer commented on PR #18528:
URL: https://github.com/apache/doris/pull/18528#issuecomment-1502886820
build all
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] freemandealer commented on pull request #18528: [fix](storage) free dict_page immediately after usage to save memory
Posted by "freemandealer (via GitHub)" <gi...@apache.org>.
freemandealer commented on PR #18528:
URL: https://github.com/apache/doris/pull/18528#issuecomment-1504948253
run buildall
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] yiguolei commented on a diff in pull request #18528: [fix](storage) free dict_page immediately after usage to save memory
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei commented on code in PR #18528:
URL: https://github.com/apache/doris/pull/18528#discussion_r1162198914
##########
be/src/olap/rowset/segment_v2/column_reader.cpp:
##########
@@ -1107,9 +1107,10 @@ Status FileColumnIterator::_read_data_page(const OrdinalPageIndexIterator& iter)
Slice dict_data;
PageFooterPB dict_footer;
_opts.type = INDEX_PAGE;
Review Comment:
If index is used, is there such case that we read page number 10 first and then read page number 1? Or in other words, could we make sure that the data page is read in descending order?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] freemandealer closed pull request #18528: [wip][fix](storage) free dict_page immediately after usage to save memory
Posted by "freemandealer (via GitHub)" <gi...@apache.org>.
freemandealer closed pull request #18528: [wip][fix](storage) free dict_page immediately after usage to save memory
URL: https://github.com/apache/doris/pull/18528
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] freemandealer commented on a diff in pull request #18528: [WIP][fix](storage) free dict_page immediately after usage to save memory
Posted by "freemandealer (via GitHub)" <gi...@apache.org>.
freemandealer commented on code in PR #18528:
URL: https://github.com/apache/doris/pull/18528#discussion_r1162427202
##########
be/src/olap/rowset/segment_v2/column_reader.cpp:
##########
@@ -1107,9 +1107,10 @@ Status FileColumnIterator::_read_data_page(const OrdinalPageIndexIterator& iter)
Slice dict_data;
PageFooterPB dict_footer;
_opts.type = INDEX_PAGE;
Review Comment:
> If index is used, is there such case that we read page number 10 first and then read page number 1? Or in other words, could we make sure that the data page is read in descending order?
@yiguolei Yes, we are sure the data page is read in ascending order: after locating the first target page, we will read the following pages sequentially, for a single query of course.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] freemandealer commented on pull request #18528: [fix](storage) free dict_page immediately after usage to save memory
Posted by "freemandealer (via GitHub)" <gi...@apache.org>.
freemandealer commented on PR #18528:
URL: https://github.com/apache/doris/pull/18528#issuecomment-1503301126
run p0
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #18528: [fix](storage) free dict_page immediately after usage to save memory
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18528:
URL: https://github.com/apache/doris/pull/18528#issuecomment-1501826464
clang-tidy review says "All clean, LGTM! :+1:"
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] freemandealer commented on a diff in pull request #18528: [fix](storage) free dict_page immediately after usage to save memory
Posted by "freemandealer (via GitHub)" <gi...@apache.org>.
freemandealer commented on code in PR #18528:
URL: https://github.com/apache/doris/pull/18528#discussion_r1161684921
##########
be/src/olap/rowset/segment_v2/column_reader.h:
##########
@@ -359,7 +359,7 @@ class FileColumnIterator final : public ColumnIterator {
std::unique_ptr<PageDecoder> _dict_decoder;
// keep dict page handle to avoid released
Review Comment:
> also update this comment
seems no problem? we do need to keep it for some 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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] zhannngchen commented on a diff in pull request #18528: [fix](storage) free dict_page immediately after usage to save memory
Posted by "zhannngchen (via GitHub)" <gi...@apache.org>.
zhannngchen commented on code in PR #18528:
URL: https://github.com/apache/doris/pull/18528#discussion_r1161679183
##########
be/src/olap/rowset/segment_v2/column_reader.h:
##########
@@ -359,7 +359,7 @@ class FileColumnIterator final : public ColumnIterator {
std::unique_ptr<PageDecoder> _dict_decoder;
// keep dict page handle to avoid released
Review Comment:
also update this comment
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #18528: [fix](storage) free dict_page immediately after usage to save memory
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18528:
URL: https://github.com/apache/doris/pull/18528#issuecomment-1501752903
clang-tidy review says "All clean, LGTM! :+1:"
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] zhannngchen commented on a diff in pull request #18528: [fix](storage) free dict_page immediately after usage to save memory
Posted by "zhannngchen (via GitHub)" <gi...@apache.org>.
zhannngchen commented on code in PR #18528:
URL: https://github.com/apache/doris/pull/18528#discussion_r1161681716
##########
be/src/olap/rowset/segment_v2/column_reader.h:
##########
@@ -359,7 +359,7 @@ class FileColumnIterator final : public ColumnIterator {
std::unique_ptr<PageDecoder> _dict_decoder;
// keep dict page handle to avoid released
- PageHandle _dict_page_handle;
+ PageHandle* _dict_page_handle = nullptr;
Review Comment:
We'd better to avoid use pointer directly, consider unique_ptr?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #18528: [fix](storage) free dict_page immediately after usage to save memory
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18528:
URL: https://github.com/apache/doris/pull/18528#issuecomment-1501942694
PR approved by at least one committer and no changes requested.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] freemandealer commented on pull request #18528: [fix](storage) free dict_page immediately after usage to save memory
Posted by "freemandealer (via GitHub)" <gi...@apache.org>.
freemandealer commented on PR #18528:
URL: https://github.com/apache/doris/pull/18528#issuecomment-1504453679
run p0
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org