Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement a fast row container reader to dump rows from disk #45125

Closed
YangKeao opened this issue Jul 3, 2023 · 0 comments · Fixed by #45130
Closed

Implement a fast row container reader to dump rows from disk #45125

YangKeao opened this issue Jul 3, 2023 · 0 comments · Fixed by #45130
Labels
affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.1 This bug affects the 7.1.x(LTS) versions. severity/major sig/sql-infra SIG: SQL Infra type/bug The issue is confirmed as a bug.

Comments

@YangKeao
Copy link
Member

YangKeao commented Jul 3, 2023

Enhancement

The current implementation of reading rows from RowContainer is not fast enough. The following function is used to dump a chunk:

// GetRowAndAppendToChunk gets a Row from the ListInDisk by RowPtr. Return the Row and the Ref Chunk.
func (l *ListInDisk) GetRowAndAppendToChunk(ptr RowPtr, chk *Chunk) (row Row, _ *Chunk, err error) {
	off, err := l.getOffset(ptr.ChkIdx, ptr.RowIdx)
	if err != nil {
		return
	}
	r := l.dataFile.getSectionReader(off)
	format := rowInDisk{numCol: len(l.fieldTypes)}
	_, err = format.ReadFrom(r)
	if err != nil {
		return row, nil, err
	}
	row, chk = format.toRow(l.fieldTypes, chk)
	return row, chk, err
}

It would be better to pipeline the format.ReadFrom (which is blocked by disk) and the format.toRow (which is blocked by CPU).

Also, with the assumption that a whole chunk is stored together tightly, we can get the first row offset and use it for the whole chunk.

@YangKeao YangKeao added the type/enhancement The issue or PR belongs to an enhancement. label Jul 3, 2023
ti-chi-bot bot pushed a commit that referenced this issue Jul 4, 2023
@YangKeao YangKeao added affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.1 This bug affects the 7.1.x(LTS) versions. type/bug The issue is confirmed as a bug. severity/major and removed type/enhancement The issue or PR belongs to an enhancement. labels Jul 10, 2023
@ti-chi-bot ti-chi-bot bot added may-affects-5.2 This bug maybe affects 5.2.x versions. may-affects-5.3 This bug maybe affects 5.3.x versions. may-affects-5.4 This bug maybe affects 5.4.x versions. may-affects-6.1 labels Jul 10, 2023
@YangKeao YangKeao added sig/sql-infra SIG: SQL Infra and removed may-affects-5.2 This bug maybe affects 5.2.x versions. may-affects-5.3 This bug maybe affects 5.3.x versions. may-affects-5.4 This bug maybe affects 5.4.x versions. may-affects-6.1 labels Jul 10, 2023
ti-chi-bot bot pushed a commit that referenced this issue Jul 10, 2023
ti-chi-bot bot pushed a commit that referenced this issue Jul 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.1 This bug affects the 7.1.x(LTS) versions. severity/major sig/sql-infra SIG: SQL Infra type/bug The issue is confirmed as a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant