Skip to content

feat(storage): serve the fast open read through wrapped configs - #14875

Draft
Dhriti07 wants to merge 1 commit into
fast-open/05-remove-prototypefrom
fast-open/06-fast-open-through-wrappers
Draft

Dhriti07 wants to merge 1 commit into
fast-open/05-remove-prototypefrom
fast-open/06-fast-open-through-wrappers

Conversation

@Dhriti07

Copy link
Copy Markdown
Contributor

6/n of fast open BiDi reads

With OpenTelemetry tracing on, readAs wraps the config in OtelReadProjectionConfig, so it never equals the fast open config and the fast open read is never handed out.

ReadProjectionConfig gets two package-private hooks: unwrap(), which a wrapper uses to return the config it wraps, and onFastOpenReadClaimed(), which is called when readAs is served from the fast open read. ObjectReadSessionImpl looks up the fast open read with config.unwrap(). OtelReadProjectionConfig unwraps to its delegate and ends its readAs span on a hit, because the bytes were already requested with the open.

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request introduces support for unwrapping wrapped ReadProjectionConfig instances and notifying when a fast open read is claimed. This is done by adding unwrap() and onFastOpenReadClaimed() methods to ReadProjectionConfig, implementing them in OtelStorageDecorator, and integrating them into ObjectReadSessionImpl. A test case has been added to verify this behavior. The review feedback suggests that the test's WrappedConfig helper class should recursively call delegate.unwrap() instead of returning delegate directly, ensuring consistency and robustness in case of nested wrapping.

Comment on lines +508 to +511
@Override
ReadProjectionConfig<ApiFuture<byte[]>> unwrap() {
return delegate;
}

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.

medium

For robustness and consistency with OtelStorageDecorator, unwrap() should recursively call delegate.unwrap() instead of returning delegate directly. This ensures that if the delegate itself is wrapped, it will be correctly unwrapped to the base config.

Suggested change
@Override
ReadProjectionConfig<ApiFuture<byte[]>> unwrap() {
return delegate;
}
@Override
ReadProjectionConfig<ApiFuture<byte[]>> unwrap() {
return delegate.unwrap();
}

Sixth step of fast open BiDi reads (go/fast-open-bidi-read, b/552263062).

With OpenTelemetry tracing on, readAs wraps the config in
OtelReadProjectionConfig, so it never equals the fast open config and the
fast open read is never handed out.

ReadProjectionConfig gets two package-private hooks: unwrap(), which a
wrapper uses to return the config it wraps, and onFastOpenReadClaimed(),
which is called with the projection when readAs is served from the fast open
read. ObjectReadSessionImpl looks up the fast open read with config.unwrap().
OtelReadProjectionConfig unwraps to its delegate. On a hit it ends its readAs
span when the read finishes, recording the error if the read fails; for a
channel it ends the span right away.
@Dhriti07
Dhriti07 force-pushed the fast-open/06-fast-open-through-wrappers branch from 3108ca7 to dedf4d1 Compare October 31, 2026 18:48

This branch has not been deployed

No deployments
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.

1 participant