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

go.mod: bump Pebble to e9a8c4ad65c5 #98078

Merged

Conversation

RaduBerinde
Copy link
Member

e9a8c4ad sstable: fix error path in writer.Close
fdf055dd db: improve documentation of tableNewIters
717cbce0 db: add Options.LoggerAndTracer for tracing
7751e381 objstorage: better Writable API
78c8001e db: add missing error check in TestIngestLoad
1334233b metamorphic: add unit test for Options propagation
ed824c76 objstorage: rename ReadaheadHandle to ReadHandle

Informs #97350.

Release note: none
Epic: none

@RaduBerinde RaduBerinde requested review from jbowens and a team March 6, 2023 18:56
@RaduBerinde RaduBerinde requested a review from a team as a code owner March 6, 2023 18:56
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Collaborator

@jbowens jbowens left a comment

Choose a reason for hiding this comment

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

Looks like there are some interface changes that we need to account for.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained

@RaduBerinde RaduBerinde force-pushed the radu/pebble-master-e9a8c4ad65c5 branch from 10a476f to 0126cad Compare March 6, 2023 22:45
@RaduBerinde RaduBerinde requested a review from a team as a code owner March 6, 2023 22:45
@RaduBerinde RaduBerinde requested a review from a team March 6, 2023 22:45
@RaduBerinde RaduBerinde requested a review from a team as a code owner March 6, 2023 22:45
@RaduBerinde RaduBerinde requested review from rhu713 and removed request for a team March 6, 2023 22:45
Copy link
Member Author

@RaduBerinde RaduBerinde left a comment

Choose a reason for hiding this comment

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

Yeah, I forgot about the writable API change. The changes were more extensive than I expected. PTAL

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @rhu713)

@RaduBerinde RaduBerinde force-pushed the radu/pebble-master-e9a8c4ad65c5 branch from 0126cad to 7186fa6 Compare March 6, 2023 23:23
Copy link
Collaborator

@nicktrav nicktrav left a comment

Choose a reason for hiding this comment

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

I took a fairly mechanical pass through the changes. :lgtm:

Reviewed 4 of 4 files at r1, 32 of 32 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @RaduBerinde and @rhu713)


pkg/storage/sst_writer.go line 461 at r2 (raw file):

}

//// Flush implements the same interface as the standard library's *bufio.Writer's

nit: dead code?

e9a8c4ad sstable: fix error path in writer.Close
fdf055dd db: improve documentation of tableNewIters
717cbce0 db: add Options.LoggerAndTracer for tracing
7751e381 objstorage: better Writable API
78c8001e db: add missing error check in TestIngestLoad
1334233b metamorphic: add unit test for Options propagation
ed824c76 objstorage: rename ReadaheadHandle to ReadHandle

The "better Writable API" changed the interface to write to SST files.
This commit contains the required changes in Cockroach: MemFile is now
MemObject and implements objstorage.Writable. There were many test
places that were using MemFile as just an io.Writer; these are updated
to just use bytes.Buffer.

Informs cockroachdb#97350.

Release note: none
Epic: none
@RaduBerinde RaduBerinde force-pushed the radu/pebble-master-e9a8c4ad65c5 branch from 7186fa6 to 90ba5a1 Compare March 6, 2023 23:43
Copy link
Member Author

@RaduBerinde RaduBerinde left a comment

Choose a reason for hiding this comment

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

TFTR!

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @nicktrav and @rhu713)


pkg/storage/sst_writer.go line 461 at r2 (raw file):

Previously, nicktrav (Nick Travers) wrote…

nit: dead code?

Fixed.

Copy link
Collaborator

@nicktrav nicktrav left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @rhu713)

@RaduBerinde
Copy link
Member Author

bors r+

@craig
Copy link
Contributor

craig bot commented Mar 7, 2023

Build succeeded:

@craig craig bot merged commit c078537 into cockroachdb:master Mar 7, 2023
@RaduBerinde RaduBerinde deleted the radu/pebble-master-e9a8c4ad65c5 branch September 19, 2023 15:59
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