[jira] [Updated] (HBASE-29630) RetryingCallerInterceptor should be wired up for batch calls

["Ray Mattingly (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/HBASE-29630?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ray Mattingly updated HBASE-29630:
----------------------------------
    Description: 
h3. Summary

{{RetryingCallerInterceptor}} should be invoked for *batch operations* ({{{}callWithoutRetries(...){}}}) in addition to single operations ({{{}callWithRetries(...){}}}). This will ensure consistent behavior for failure handling, fast-fail logic, and custom metrics across all client operations.
h3. Problem
 * *Single operations* ({{{}HTable.get(Get){}}}, etc.) run through {{{}RpcRetryingCaller.callWithRetries(){}}}, which wires in interceptor hooks ({{{}intercept(){}}}, {{{}handleFailure(){}}}, {{{}updateFailureInfo(){}}}).

 * *Batch operations* ({{{}HTable.batch(){}}}, {{{}HTable.get(List<Get>){}}}) go through {{{}RpcRetryingCaller.callWithoutRetries(){}}}, which currently bypasses interceptor hooks.

 * As a result:

 * 
 ** Custom interceptors (e.g., handling {{{}RpcThrottlingException{}}}) do not run for batch calls.

 * 
 ** Metrics and fast-fail logic only apply to single operations, not batch operations.

 * 
 ** Behavior is inconsistent between single and batch APIs.

h3. Proposed Change

Update {{RpcRetryingCallerImpl.callWithoutRetries(...)}} to use the same interceptor lifecycle as {{{}callWithRetries(...){}}}:
 # Create an interceptor context at call start.
 # Call {{interceptor.intercept()}} before executing the RPC.
 # On exception:
 ## Translate the throwable.
 ## Pass the translated exception into {{{}interceptor.handleFailure(){}}}.
 ## Preserve {{PreemptiveFastFailException}} behavior.
 # Call {{interceptor.updateFailureInfo()}} in the {{finally}} block when failures occur.
 # Add an {{AttemptType.NO_RETRY}} marker to the context for clarity in logs/metrics.
 # Optionally enrich the context when {{RetriesExhaustedWithDetailsException}} (REWDE) is thrown, so interceptors can see per-item failures.

h3. Benefits
 * *Consistency:* Single and batch operations follow the same interceptor pattern.

 * *Extensibility:* Interceptors can handle throttling and emit metrics for batch calls without custom forks.

 * *Maintainability:* Small, contained change in {{{}RpcRetryingCallerImpl{}}}; no invasive changes to {{{}AsyncProcess{}}}.

 * *Compatibility:* No breaking API changes. No-op interceptors behave as before.

 * *Community value:* Opens the door to richer failure handling for all client operations.

h3. Example Use Case

At my day job we use interceptors to detect {{RpcThrottlingException}} and fail fast while reporting metrics. This works for single gets/puts but is currently bypassed in batch calls, limiting visibility and resilience. With this change, batch requests can also fast-fail and report throttling metrics.
h3. Risks & Mitigation
 * *Performance regression:* Interceptor calls are lightweight; validate with before/after microbenchmarks.

 * *Behavior change for existing interceptors:* Only makes interceptors {_}more consistent{_}. Default no-op interceptor ensures compatibility.

 * *Naming confusion (“RetryingCaller” on no-retry path):* Clarify with Javadoc: interceptors apply to both retrying and no-retry paths.

  was:
h3. Summary

{{RetryingCallerInterceptor}} should be invoked for *batch operations* ({{{}callWithoutRetries(...){}}}) in addition to single operations ({{{}callWithRetries(...){}}}). This will ensure consistent behavior for failure handling, fast-fail logic, and custom metrics across all client operations.
h3. Problem
 * *Single operations* ({{{}HTable.get(Get){}}}, etc.) run through {{{}RpcRetryingCaller.callWithRetries(){}}}, which wires in interceptor hooks ({{{}intercept(){}}}, {{{}handleFailure(){}}}, {{{}updateFailureInfo(){}}}).

 * *Batch operations* ({{{}HTable.batch(){}}}, {{{}HTable.get(List<Get>){}}}) go through {{{}RpcRetryingCaller.callWithoutRetries(){}}}, which currently bypasses interceptor hooks.

 * As a result:

 ** Custom interceptors (e.g., handling {{{}RpcThrottlingException{}}}) do not run for batch calls.

 ** Metrics and fast-fail logic only apply to single operations, not batch operations.

 ** Behavior is inconsistent between single and batch APIs.

h3. Proposed Change

Update {{RpcRetryingCallerImpl.callWithoutRetries(...)}} to use the same interceptor lifecycle as {{{}callWithRetries(...){}}}:
 # Create an interceptor context at call start.

 # Call {{interceptor.intercept()}} before executing the RPC.

 # On exception:

 ** Translate the throwable.

 ** Pass the translated exception into {{{}interceptor.handleFailure(){}}}.

 ** Preserve {{PreemptiveFastFailException}} behavior.

 # Call {{interceptor.updateFailureInfo()}} in the {{finally}} block when failures occur.

 # Add an {{AttemptType.NO_RETRY}} marker to the context for clarity in logs/metrics.

 # Optionally enrich the context when {{RetriesExhaustedWithDetailsException}} (REWDE) is thrown, so interceptors can see per-item failures.

h3. Benefits
 * *Consistency:* Single and batch operations follow the same interceptor pattern.

 * *Extensibility:* Interceptors can handle throttling and emit metrics for batch calls without custom forks.

 * *Maintainability:* Small, contained change in {{{}RpcRetryingCallerImpl{}}}; no invasive changes to {{{}AsyncProcess{}}}.

 * *Compatibility:* No breaking API changes. No-op interceptors behave as before.

 * *Community value:* Opens the door to richer failure handling for all client operations.

h3. Example Use Case

At my day job we use interceptors to detect {{RpcThrottlingException}} and fail fast while reporting metrics. This works for single gets/puts but is currently bypassed in batch calls, limiting visibility and resilience. With this change, batch requests can also fast-fail and report throttling metrics.
h3. Risks & Mitigation
 * *Performance regression:* Interceptor calls are lightweight; validate with before/after microbenchmarks.

 * *Behavior change for existing interceptors:* Only makes interceptors {_}more consistent{_}. Default no-op interceptor ensures compatibility.

 * *Naming confusion (“RetryingCaller” on no-retry path):* Clarify with Javadoc: interceptors apply to both retrying and no-retry paths.


> RetryingCallerInterceptor should be wired up for batch calls
> ------------------------------------------------------------
>
>                 Key: HBASE-29630
>                 URL: https://issues.apache.org/jira/browse/HBASE-29630
>             Project: HBase
>          Issue Type: Improvement
>    Affects Versions: 2.6.3
>            Reporter: Ray Mattingly
>            Assignee: Ray Mattingly
>            Priority: Major
>
> h3. Summary
> {{RetryingCallerInterceptor}} should be invoked for *batch operations* ({{{}callWithoutRetries(...){}}}) in addition to single operations ({{{}callWithRetries(...){}}}). This will ensure consistent behavior for failure handling, fast-fail logic, and custom metrics across all client operations.
> h3. Problem
>  * *Single operations* ({{{}HTable.get(Get){}}}, etc.) run through {{{}RpcRetryingCaller.callWithRetries(){}}}, which wires in interceptor hooks ({{{}intercept(){}}}, {{{}handleFailure(){}}}, {{{}updateFailureInfo(){}}}).
>  * *Batch operations* ({{{}HTable.batch(){}}}, {{{}HTable.get(List<Get>){}}}) go through {{{}RpcRetryingCaller.callWithoutRetries(){}}}, which currently bypasses interceptor hooks.
>  * As a result:
>  * 
>  ** Custom interceptors (e.g., handling {{{}RpcThrottlingException{}}}) do not run for batch calls.
>  * 
>  ** Metrics and fast-fail logic only apply to single operations, not batch operations.
>  * 
>  ** Behavior is inconsistent between single and batch APIs.
> h3. Proposed Change
> Update {{RpcRetryingCallerImpl.callWithoutRetries(...)}} to use the same interceptor lifecycle as {{{}callWithRetries(...){}}}:
>  # Create an interceptor context at call start.
>  # Call {{interceptor.intercept()}} before executing the RPC.
>  # On exception:
>  ## Translate the throwable.
>  ## Pass the translated exception into {{{}interceptor.handleFailure(){}}}.
>  ## Preserve {{PreemptiveFastFailException}} behavior.
>  # Call {{interceptor.updateFailureInfo()}} in the {{finally}} block when failures occur.
>  # Add an {{AttemptType.NO_RETRY}} marker to the context for clarity in logs/metrics.
>  # Optionally enrich the context when {{RetriesExhaustedWithDetailsException}} (REWDE) is thrown, so interceptors can see per-item failures.
> h3. Benefits
>  * *Consistency:* Single and batch operations follow the same interceptor pattern.
>  * *Extensibility:* Interceptors can handle throttling and emit metrics for batch calls without custom forks.
>  * *Maintainability:* Small, contained change in {{{}RpcRetryingCallerImpl{}}}; no invasive changes to {{{}AsyncProcess{}}}.
>  * *Compatibility:* No breaking API changes. No-op interceptors behave as before.
>  * *Community value:* Opens the door to richer failure handling for all client operations.
> h3. Example Use Case
> At my day job we use interceptors to detect {{RpcThrottlingException}} and fail fast while reporting metrics. This works for single gets/puts but is currently bypassed in batch calls, limiting visibility and resilience. With this change, batch requests can also fast-fail and report throttling metrics.
> h3. Risks & Mitigation
>  * *Performance regression:* Interceptor calls are lightweight; validate with before/after microbenchmarks.
>  * *Behavior change for existing interceptors:* Only makes interceptors {_}more consistent{_}. Default no-op interceptor ensures compatibility.
>  * *Naming confusion (“RetryingCaller” on no-retry path):* Clarify with Javadoc: interceptors apply to both retrying and no-retry paths.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)