morningman commented on PR #68711: URL: https://github.com/apache/doris/pull/68711#issuecomment-5976705411
Addressed in bb33f27a1ad. Per finding of the review: - M1 (a reused connection reads with another catalog's filesystem settings): no code change. BE's fluss plugin loads only fluss's local filesystem, which ignores `client.fs.*`, and fluss's process-wide `FileSystem` configuration was already shared between catalogs on master; details in the thread. - M2 (connections lost when an `OutOfMemoryError` strikes a sweep after taking them out): fixed. The sweep hands connections to the closer one at a time and logs afterwards, and the closer closes a connection on the calling thread when it cannot hand it to a closer thread. New test `FlussConnectionPoolTest.sweepThatFailsMidwayKeepsTheConnectionsItHadNotHandedOver`, which fails without the change. `borrow()` and `giveBack()` stay as they are; details in the thread. - M3 (no cap on idle connections across configurations): the javadoc that claimed a pool-wide bound now states the per-configuration one. No global cap; details in the thread. - M4 (the deferred hand-back of a `PK_FULL` range's connection depends on the publication waiter): fixed. The waiter outlasts an `OutOfMemoryError` on its own thread, a failure to build its thread falls back as a failure to start it does, and the deferred discard no longer throws. New test `SafeKvSnapshotAndLogBatchScannerTest.publicationWaiterOutlastsAnOutOfMemoryErrorOnItsOwnThread`, which fails without the change. The remark that the BE test checks the shared cache's wiring rather than a hit needs no change: the commit moves the cache's ownership to the operator, which is what the test checks, and the cache's own logic is untouched. fluss-scanner: 79 tests pass. The description is updated (commit list, unit tests). -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
