Repository navigation
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the Bigtable client's session diversion and shim layers, introducing support for global and per-scope session diversion configurations in DivertingUnaryCallable and refactoring ReadRowShim and ReadModifyWriteRowShim into stateless wrappers around inner stateful shims. However, the refactoring in ShimImpl.java introduced several compilation errors. Specifically, ReadRowShimInner is not imported, the field declarations do not match their initializations in the constructor, and decorateReadRow incorrectly attempts to use the removed readRowShim field instead of instantiating the new generic ReadRowShim class.
Add a methodScopeKey to DivertingUnaryCallable so session load can be configured per-RPC-method via SessionDiversionConfiguration, in addition to the existing global session_load. Resolve load from global_diversion or per_scope_diversion based on which strategy the server returns. Also move the supports() check before resolveSessionLoad() to avoid a config lookup for requests the shim can't handle, and fix ClientConfigurationManager to preserve session config when a diversion configuration is present even if the global session_load is 0. Change-Id: I3925b50b9ce0bd4e7b91b17888f9745d07dd9919
56b775f to
cc0ad8f
Compare
mutianf
changed the title
…e.useExperimental Change-Id: I0501f16a70e7de8c3d9b7df3cb8dcb95040ab07c
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for session diversion configurations in DivertingUnaryCallable, enabling both global and per-scope diversion strategies (e.g., routing specific Bigtable methods like ReadRow or MutateRow based on a per-method session load). It also updates ClientConfigurationManager to preserve session configurations when a diversion configuration is present and adds comprehensive unit tests to verify the routing behavior. There are no review comments provided, and the changes look solid, so I have no additional feedback to provide.
This branch has not been deployed
1 participant
Summary
methodScopeKeytoDivertingUnaryCallableso session load can be configured per-RPC-method viaSessionDiversionConfiguration, in addition to the existing globalsession_load.resolveSessionLoad()checksglobal_diversionorper_scope_diversiondepending on which strategy the server returns; falls back to flatsession_loadwhen no diversion config is present.supports()check beforeresolveSessionLoad()to avoid a config lookup for requests the shim can't handle.ClientConfigurationManagerto preserve session config when aSessionDiversionConfigurationis present even if the globalsession_loadis 0 (diversion config encodes its own per-scope session loads).Test plan
DivertingUnaryCallableTest: covers global diversion (load=1 routes to experimental, load=0 falls back to classic), per-scope match (routes to experimental), and per-scope non-match (falls back to classic)