mxtymoshyk commented on issue #28025: URL: https://github.com/apache/beam/issues/28025#issuecomment-5483614095
Still open, and I went digging into it. I think the diagnosis is a little different from "failed connection never recovered" -- the connection may well be healthy and the scan may well be making progress. The problem is that Beam has no way to tell. `advance()` (`HBaseIO.java:562`) blocks inside `iter.hasNext()`, which lands in HBase's `ClientScanner.loadCache()`. Reading that method in 2.6.5, the version `sdks/java/io/hbase/build.gradle` pins today: The individual RPC is bounded. `loadCache` issues each request through `call(callable, caller, scannerTimeout, true)` (`ClientScanner.java:461`), which goes on to `callWithoutRetries` with `hbase.client.scanner.timeout.period`, 60s by default. Both polls inside `ScannerCallableWithReplicas.call` are bounded too, and the second one throws `"Failed to get result within timeout"` rather than waiting. So no single request can sit there forever. What is unbounded is the loop around it. `loadCache` is a bare `for (;;)` at `ClientScanner.java:453`, and `retriesLeft` only decrements on the `DoNotRetryIOException` path at `:474`. The way through it without touching any counter is the heartbeat path at `:515`. When the region server is still scanning but has not matched a row yet, it returns a heartbeat message; if the client-side cache is empty, the loop falls through to the bottom and goes around again. `countdown` is untouched because no rows arrived, `remainingResultSize` is untouched for the same reason, and `retriesLeft` never came into play because nothing threw. A scan with a selective filter over a large region can stay in there for as long as the server keeps looking, with every RPC completing successfully inside its timeout the whole time. The stack in the description is a single sample, so landing in the poll is consistent with going around that loop repeatedly rather than with one call hanging. The awkward consequence is that no HBase client setting bounds this. HBaseIO hands the user's `Configuration` straight to `ConnectionFactory.createConnection` (`HBaseIO.java:546`) and the module sets no timeouts of its own anywhere, so `hbase.client.scanner.timeout.period` and `hbase.client.retries.number` are already reachable by users. They just do not apply to this case. Worth knowing before anyone offers them as the workaround. From HBase's side nothing is going wrong here. Heartbeats exist precisely so that a slow server-side scan does not trip the scanner timeout. The problem sits on Beam's side of the boundary: while `hasNext()` is in that loop the reader cannot report progress, cannot be split, and cannot be cancelled, which is what the "operation ongoing ... without outputting or completing" message is really describing. On the fix itself, a timeout in `advance()` is possible but more constrained than it first looks. `advance()` returning false means end of input, so a timeout has no way to say "still working" -- it would have to throw and fail the bundle. That is a real behaviour change, since a genuinely slow but healthy scan would start failing, and it needs a thread per reader to interrupt. HBase RPC threads do not reliably honour interrupts, so cancelled work can be left running behind it. There is a second option I would put ahead of that, because HBase already has an answer for this exact situation. `Scan.setNeedCursorResult(true)` makes the server return a cursor `Result` on heartbeat instead of nothing, and the client then breaks out of the loop at `ClientScanner.java:528-531` instead of spinning. `advance()` would need to recognise `Result.isCursor()`, skip it as a non-row, and use `getCursor().getRow()` to move the range tracker forward. Beam gets control back on every heartbeat, which restores progress reporting and cancellation without changing when a read is allowed to fail. I checked and all three of those APIs are present in the shaded client we pin today. The catch is that it needs an HBase 2.0+ server, so it would have to be opt-in and would quietly do nothing against an older one. Those two are not the same fix and I do not want to guess which you want: one bounds how long a read may take, the other bounds how long Beam is blind for. They compose fine if you want the cursor for control and a timeout as a backstop, but that is a bigger change than either alone. Testing looks cheap whichever way it goes. `HBaseIOCloseTest` already mocks `ResultScanner` and `Connection`, so a blocking mock iterator would give a deterministic test without needing the mini cluster that `HBaseIOTest` spins up. Happy to write whichever you prefer. Not claiming the issue in the meantime. -- 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]
