prashantwason opened a new pull request, #20221:
URL: https://github.com/apache/hudi/pull/20221

   ### Describe the issue this Pull Request addresses
   
   The timeline server re-syncs its file system view on **every** request when 
an async clean completes while a write is in progress. Under a large writer 
this serializes all requests behind full timeline reloads and makes executors 
fail with `NoHttpResponseException`.
   
   **Scenario (seen in production on a COPY_ON_WRITE upsert with ~1000 
executors):**
   
   1. Writer starts commit `C1` and builds its timeline (last completed instant 
`C0`, timeline hash `H_old`). This timeline is shipped to all executors; every 
executor request to the timeline server carries `lastinstantts=C0, 
timelinehash=H_old`.
   2. An async clean `CL` (e.g. scheduled by a table service manager) completes 
while `C1` is being written.
   3. A request carrying a newer client timeline makes the timeline server sync 
its view. The server's timeline now contains `CL`.
   4. From then on, for every executor request `isLocalViewBehind` compares 
`H_old` with the server's hash. They never match again because the server has 
`CL` and the client does not. So **every request** calls `view.sync()`, which 
reloads the active timeline from storage, while holding `synchronized (view)`. 
Syncing again cannot make the hashes match.
   5. Hundreds of concurrent requests queue behind that lock and the server's 
request threads are exhausted. Marker creation 
(`TimelineServerBasedWriteMarkers.create`) hangs for ~10 minutes and fails:
   
   ```
   HoodieRemoteException: Failed to create marker file 
<partition>/<fileId>_..._<C1>.parquet.marker.MERGE
   <driver-host>:<port> failed to respond
   Caused by: org.apache.http.NoHttpResponseException: <driver-host>:<port> 
failed to respond
   ```
   
   The driver log shows the same line for every request:
   
   ```
   Syncing view as client passed last known instant C0 as last known instant 
but server has the following last instant on timeline: 
Option{val=[CL__clean__COMPLETED]}
   ```
   
   Tasks exhaust their retries and the write stage aborts. The post-request 
check `shouldThrowExceptionIfLocalViewBehind` already tolerates this exact case 
(a trailing completed clean), so the requests themselves would succeed. Only 
the repeated syncs are the problem.
   
   ### Summary and Changelog
   
   For a given stale client timeline, the timeline server now syncs its view 
once instead of once per request.
   
   A client's timeline hash describes a timeline it loaded from storage before 
sending the request. Once the server has synced its view after receiving a 
request carrying hash `H`, the server's view is at least as recent as any 
client still holding `H`. Further syncs for `H` are redundant, so the server 
remembers `H` and skips them.
   
   - `RequestHandler`: added `syncedClientTimelineHashes`, which maps each file 
system view to the set of client timeline hashes that view has already been 
synced past.
     - The map is keyed by view instance with weak keys. A view that is cleared 
and re-created starts empty, so its first sync is never skipped, and entries go 
away with dropped views. The map is synchronized; each per-view set is only 
touched while holding that view's lock, which `syncIfLocalViewBehind` already 
takes.
     - Each per-view set is bounded (`MAX_SYNCED_CLIENT_TIMELINE_HASHES = 32`) 
and backed by an insertion-ordered `LinkedHashMap` that evicts the oldest entry 
(FIFO). Evicting a hash that is still in use only costs one extra sync for it; 
correctness is unaffected.
     - `syncIfLocalViewBehind` skips the sync when the client's hash is in the 
view's set, and records the hash after a sync. Requests without a timeline hash 
are never skipped.
   - Added a `VIEW_SYNC` counter to the `TimelineService` metrics registry, so 
redundant syncs are observable.
   - `TestRequestHandler#testViewSyncedOnceForStaleClientTimeline` reproduces 
the scenario: commit, stale client timeline, a clean completes, the server 
syncs for the new timeline, then 32 concurrent requests with the stale hash. It 
asserts exactly one sync. Without the fix the test fails with `expected: <1> 
but was: <32>`.
   
   ### Impact
   
   Performance and stability of the timeline server only. There are no API, 
config, or storage format changes. Writers whose executors carry a timeline 
that predates a concurrently completed clean (or any other instant) no longer 
trigger a full timeline reload per request.
   
   ### Risk Level
   
   low
   
   A sync is skipped only when the same view has already been synced after a 
request carrying the exact same client timeline hash was received, which 
guarantees the view is at least as recent as that client. New views, new 
hashes, and requests without a hash behave as before. The post-request check 
(`shouldThrowExceptionIfLocalViewBehind`) is unchanged. All 
`hudi-timeline-service` tests pass, including 
`TestRemoteHoodieTableFileSystemView`.
   
   ### Documentation Update
   
   none
   
   ### Contributor's checklist
   
   - [x] Read through [contributor's 
guide](https://hudi.apache.org/contribute/how-to-contribute)
   - [x] Enough context is provided in the sections above
   - [x] Adequate tests were added if applicable
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to