Skip to content

Limit V2 segment file row count (#1647) - #1705

Merged
chaoyli merged 4 commits into
apache:masterfrom
liutang123:list-segment-file-size
Aug 30, 2019
Merged

chaoyli merged 4 commits into
apache:masterfrom
liutang123:list-segment-file-size

Conversation

@liutang123

Copy link
Copy Markdown
Contributor

Because many bitmap implementations usually only support 32 bits integer,
so use INT_MAX to limit the row count of a segment file.

@liutang123

liutang123 commented Aug 27, 2019 •

Copy link
Copy Markdown
Contributor Author

cc @gaodayue

if (PREDICT_FALSE(_segment_writer == nullptr)) {
RETURN_NOT_OK(_create_segment_writer());
}
if (PREDICT_FALSE(_segment_writer->reach_capacity())) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can check with _max_segment_size together.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1


uint64_t estimate_segment_size();

bool reach_capacity();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to have a function named num_rows_written() and move the logic of when to flush segment to BetaRowsetWriter.

std::unique_ptr<WritableFile> _output_file;
std::vector<ColumnWriter*> _column_writers;
uint64_t _row_count = 0;
const int32_t MAX_ROW_COUNT = INT_MAX;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

INT_MAX is not the same on different platforms, better to use INT32_MAX.

if (PREDICT_FALSE(_segment_writer == nullptr)) {
RETURN_NOT_OK(_create_segment_writer());
}
if (PREDICT_FALSE(_segment_writer->reach_capacity())) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@gaodayue

Copy link
Copy Markdown
Contributor

In addition

  • SegmentFooterPB.num_rows should be changed from uint64 to uint32
  • segment_v2::SegmentWriter::_row_count should be changed from uint64_t to uint32_t

@liutang123
liutang123 force-pushed the list-segment-file-size branch from cfd9f36 to 612458d Compare August 30, 2019 09:13

@gaodayue gaodayue left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@chaoyli

chaoyli commented Aug 30, 2019

Copy link
Copy Markdown
Contributor

LGTM, I will merge it after code checks.


uint64_t estimate_segment_size();

uint32_t num_rows_written() { return _row_count; }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
uint32_t num_rows_written() { return _row_count; }
uint32_t num_rows_written() const { return _row_count; }

@chaoyli
chaoyli merged commit 206f539 into apache:master Aug 30, 2019
swjtu-zhanglei pushed a commit to swjtu-zhanglei/incubator-doris that referenced this pull request Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants