LompleZ commented on PR #64224: URL: https://github.com/apache/doris/pull/64224#issuecomment-4669046151
**** > I found blocking correctness issues in the broker file-system cache refactor. > > Critical checkpoint conclusions: > > * Goal/test: The PR aims to close evicted file systems safely without breaking active users, but the new cleanup logic can still leave recycled file systems unclosed, and no tests were added to cover the new reference-count/recycle-bin behavior or error-code preservation. > * Scope: The change is focused on broker FS caching, but it broadens several catch blocks and removes the per-stream expiration path, which changes behavior beyond the cache close timing. > * Concurrency/lifecycle: The active operation/stream counters are intended to protect concurrent use, but leaked active streams can now keep recycled file systems alive indefinitely while the client context stays active. > * Configuration: Existing input-stream expiration configuration was removed; this removes the only per-stream cleanup control. > * Compatibility/storage format: No storage/protocol format compatibility concerns found. > * Parallel paths: The broad catch pattern is repeated across several path operations and should be fixed consistently. > * Test coverage: Missing targeted tests for logical BrokerException propagation and recycle-bin cleanup when streams expire or are leaked. > * Observability/performance: Cleanup logs exist, but repeated INFO logs every 3 seconds for still-active recycled FSs may be noisy; secondary to the blocking issues. > * Transaction/data correctness: Not applicable to this broker FS cache change. > > User focus: No additional user-provided review focus was present. -- 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]
