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

add WriteBatch #26

Merged
merged 12 commits into from
Sep 14, 2023
Merged

add WriteBatch #26

merged 12 commits into from
Sep 14, 2023

Conversation

akiozihao
Copy link
Contributor

This approach may have lower efficiency for short values and can potentially disrupt the segment when failures occur, requiring more testing.

@amityahav
Copy link
Contributor

Hey @akiozihao. rose already came up with some ideas around this feature in #23 . dont u prefer taking this path? it is also similar to tidwal's WAl, except for creating a seperate struct for each batch.
also i've noticed that when writing the batch, you simply iterate over all records and initiate a write each time. it can be optimized if you first create an in-memory buffer that will hold all records concatenated together where each record already was enriched with the CRC and all the other metadata. and then write this buffer once. (writing multiple distinct records at once is not implemented either)

@akiozihao
Copy link
Contributor Author

@amityahav Thank you for your reminder; I have noticed this issue and this problem. I am currently attempting to split write into calPosition, writeToBuf to reduce redundant code. There are many details to consider, and I will submit updates as soon as possible.

wal.go Outdated
wal.mu.Lock()
defer wal.mu.Unlock()

if wal.pendingSize+wal.calSizeUpperBound(int64(len(data))) > wal.options.SegmentSize {
Copy link
Contributor

Choose a reason for hiding this comment

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

wal.calSizeUpperBound can be put in a variable and reused in 311 and 314

segment.go Outdated
}

// WriteAll write batch data to the segment file.
func (seg *segment) WriteAll(data [][]byte) ([]*ChunkPosition, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can u maybe privatize all segment functions that are invoked from within wal's methods? such as WriteAll, Write, Read.. as they arent supposed to be used independently. #22

segment.go Outdated
// You can call Next to get the next chunk data,
// and io.EOF will be returned when there is no data.
func (seg *segment) NewReader() *segmentReader {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please split this into a single PR
For better review.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@roseduan roseduan merged commit aa0af20 into rosedblabs:main Sep 14, 2023
@akiozihao akiozihao deleted the batch branch September 14, 2023 01:51
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.

3 participants