Doris-Breakwater commented on issue #66300: URL: https://github.com/apache/doris/issues/66300#issuecomment-5130058674
## Initial maintainer analysis **Assessment:** the unbounded retry and repeated-I/O amplification are confirmed bugs, and failed multipart uploads do leave remote multipart state behind. However, the proposed file-descriptor causal chain is not yet established and contains two important inaccuracies. I recommend keeping this open as a backup reliability bug, while requesting the runtime evidence below before attributing the node outages to a BE socket/FD leak. I checked `4.1.0-rc03` and local current master (`27cb451229c7a42dd089c5248847828eb0536d02`). The relevant behavior is unchanged on master. ### Confirmed from code 1. A failed snapshot `UPLOAD` task is retried without a task-level cap or backoff. On a non-OK completion, [`finishSnapshotUploadTask()` records the error and returns `false`](https://github.com/apache/doris/blob/4.1.0-rc03/fe/fe-core/src/main/java/org/apache/doris/backup/BackupJob.java#L342-L348), so `MasterImpl` does not remove the task from `AgentTaskQueue`. The retry is actually driven by [`ReportHandler.taskReport()`](https://github.com/apache/doris/blob/4.1.0-rc03/fe/fe-core/src/main/java/org/apache/doris/master/ReportHandler.java#L739-L752): after the BE removes the completed/failed signature from its running-task set, the next task report sees a diff and resubmits it. BE task reports default to [10 seconds](https://github.com/apache/doris/blob/4.1.0-rc03/be/src/common/config.cpp#L258), and `UploadTask` inherits `createTime = -1`, making [`shouldResend()` immediately true](https://github.com/apache/doris/blob/4.1.0-rc03/fe/fe-core/src/main/java/org/apache/doris/task/AgentTa sk.java#L159-L161). `failedTimes` is incremented but is not used to limit this path. The job remains in `UPLOADING` until all signatures finish or the overall job deadline expires (up to the remaining portion of the default 24-hour timeout). The issue's cited `BackupJob.java:509-511` comment is about the later `UPLOAD_INFO` metadata-file stage, not snapshot `UPLOAD` task retries. `BackupJob.java:457` is the overall timeout check, not the redispatch point. 2. Every redispatch re-enters [`SnapshotLoader::upload()`](https://github.com/apache/doris/blob/4.1.0-rc03/be/src/runtime/snapshot_loader.cpp#L785-L878) from the beginning. For every snapshot directory reached before the next failure, it lists remote files, lists local files, and recomputes MD5 for every local file. A task contains at most 10 snapshot directories by default. Thus a failure late in a task can repeatedly re-read almost the full task; a failure early in the task does not necessarily re-read all ~16k files. This is nevertheless a real and potentially severe disk/CPU/network amplification. 3. [`S3FileWriter` waits for submitted part tasks but deliberately does not abort](https://github.com/apache/doris/blob/4.1.0-rc03/be/src/io/fs/s3_file_writer.cpp#L76-L93). If multipart creation succeeded and upload/complete later failed, the upload ID and uploaded parts remain in the object store until an external lifecycle rule or cleanup removes them. Adding a best-effort BE abort is appropriate for this non-committer path. 4. A generic FE metric already counts these resends: `agent_task_resend_total` with `task="UPLOAD"` is incremented at the redispatch point. It is not backup/job-specific, so a dedicated low-cardinality backup retry metric would still improve diagnosis. ### What the current code does not support - A normal S3 repository with no user-specified connection limit should not reach the BE fallback of `102400`. In `4.1.0-rc03`, [`S3Properties.maxConnections` defaults to `50`](https://github.com/apache/doris/blob/4.1.0-rc03/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/S3Properties.java#L118-L123), and the normalized repository path [always emits `AWS_MAX_CONNECTIONS`](https://github.com/apache/doris/blob/4.1.0-rc03/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/AbstractS3CompatibleProperties.java#L101-L113) to the BE. Other S3-compatible property classes also have bounded defaults. The `102400` fallback in `s3_util.cpp` is real and is unsafe for callers that omit the property, but the reproduction needs to demonstrate that the repository path actually omitted or bypassed normalization. - `maxConnections` is a ceiling on concurrent HTTP connections, not an eager allocation of that many descriptors. On the default BE configuration, backup has one `UPLOAD` worker, multipart requests run through a global S3 upload pool capped at 64 threads, and `S3ClientFactory` caches/reuses a client by configuration hash across retry-created `S3FileSystem` objects. Therefore retries do not create a fresh 102400-sized client pool each cycle. - `AbortMultipartUpload` cleans server-side multipart parts/state; it is not what releases completed client HTTP requests or local file descriptors. The S3 protocol does not hold a BE socket open for the lifetime of an upload ID. Not aborting explains remote orphaned multipart state and storage consumption, but by itself does not explain monotonically growing BE FDs. Likewise, repeated MD5 opens cause I/O churn, but the local readers are RAII-managed and should close after each checksum. So the code proves an unbounded retry/amplification defect and an object-store cleanup defect. It does **not** yet prove the monotonic FD leak. A transport/SDK/endpoint interaction or stuck-request leak is still possible and should be investigated as a suspected bug if the growing descriptors are confirmed to be S3 sockets. ### Information needed to identify the FD root cause Please provide the following, with credentials, bucket names, hostnames, and customer data redacted: 1. The original `CREATE REPOSITORY` properties and the BE log line around `create one s3 client with ... max_connections=...` for this repository. This establishes whether the effective value was 50/another bounded provider default or 102400. 2. Timestamped FD inventories at baseline and after several retry cycles, grouped by descriptor type and target (`lsof -nP -p <be_pid>` or `/proc/<pid>/fd` symlink counts). For sockets, include TCP state and redacted remote endpoint/port from `ss`. The key distinction is regular snapshot files versus sockets, and for sockets `ESTABLISHED` versus `CLOSE_WAIT`/other states. 3. Exact BE build commit and effective values for `ulimit -n`, `upload_worker_count`, `num_s3_file_upload_thread_pool_{min,max}_thread`, `s3_write_buffer_size`, connection/request timeouts, and any non-default task-report interval. 4. The matching FE resend logs/counter delta and BE upload failure logs (including error/exception names and request IDs), plus time series for `s3_file_writer_file_being_written`, `s3_file_buffer_allocated`, and the `S3FileUploadThreadPool` running/queued metrics. 5. Counts of incomplete multipart uploads before and after the reproduction. This separately verifies the confirmed remote cleanup problem. ### Recommended implementation direction 1. Add upload-specific retry state with a last-attempt timestamp, exponential backoff with jitter, and a configurable consecutive-failure cap. `AgentTask.failedTimes` already captures failed completions, but `UploadTask` currently bypasses the generic resend wait and no last-dispatch time is updated. Preserve FE crash/replay behavior and explicitly test restart during `UPLOAD_SNAPSHOT`/`UPLOADING`; retry state must not accidentally turn crash recovery into immediate cancellation. 2. Add `abort_multipart_upload` to the BE object-storage abstraction and invoke it best-effort after all in-flight part tasks have finished when an upload ID exists and the writer is not using the S3 committer. Preserve the original upload error if abort also fails, and test upload-part failure, complete failure, small `PutObject`, and committer behavior. 3. Bound retry I/O independently: checkpoint completed snapshot directories/files or cache immutable snapshot checksums for the job. As an immediate operational containment, lowering `backup_upload_snapshot_batch_size` to `1` limits how much earlier work one failed task re-lists/re-hashes. 4. Review the BE `102400` fallback separately and align it with real BE concurrency/FD budgets, but do not treat that change as the demonstrated FD-leak fix until the effective repository value and descriptor types are confirmed. 5. Keep the existing generic resend counter and add a backup-specific aggregate counter by outcome/reason (avoiding job-ID labels). Operationally, cancel a persistently failing backup promptly, set `s3.connection.maximum` explicitly within the BE FD budget, alert on `agent_task_resend_total{task="UPLOAD"}`, and configure the object store's incomplete-multipart lifecycle cleanup. The lifecycle rule mitigates orphaned remote parts; it does not address the retry storm or a BE-side FD leak. Breakwater-GitHub-Analysis-Slot: slot_7fa993599347 -- 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]
