Doris-Breakwater commented on issue #67544:
URL: https://github.com/apache/doris/issues/67544#issuecomment-5555025282

   Breakwater-GitHub-Analysis-Slot: slot_13643ad43aaa
   
   ## Initial triage
   
   **Assessment: confirmed Iceberg write-path bug with high availability 
impact.** A successful row-level DML commit can make the table's current 
snapshot unreadable until it is rolled back. The live issue currently has no 
labels; `area/iceberg` is appropriate.
   
   I verified the reported data flow against both the 4.1.3 commit 
(`7126cf65d96`) and the cited master commit (`b58b2c53ff5`). No additional 
BE/FE log or profile is needed to establish the initial root cause.
   
   ### Verified facts
   
   - FE keeps two forms of the data location: the normalized BE I/O path and 
the raw Iceberg path. For standalone DELETE, the raw path is already carried in 
`TIcebergDeleteSink.table_location`; for UPDATE/MERGE it is carried in both 
`TIcebergMergeSink.original_output_path` (data side) and `table_location` 
(delete side). Master has the same arrangement: [`buildDeleteSink()` sets 
normalized `output_path` and raw 
`table_location`](https://github.com/apache/doris/blob/b58b2c53ff56354c692c2dc7634d724d8780838f/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergWritePlanProvider.java#L804-L834).
   - BE reads both delete-sink fields, but `_generate_delete_file_path()` 
always prefers non-empty `_output_path`. It then gives that single normalized 
path to `VIcebergDeleteFileWriter`, which uses it for physical I/O and also 
returns it through `commit_data.file_path`: 
[`viceberg_delete_sink.cpp`](https://github.com/apache/doris/blob/b58b2c53ff56354c692c2dc7634d724d8780838f/be/src/exec/sink/viceberg_delete_sink.cpp#L482-L511),
 
[`viceberg_delete_file_writer.cpp`](https://github.com/apache/doris/blob/b58b2c53ff56354c692c2dc7634d724d8780838f/be/src/exec/sink/writer/iceberg/viceberg_delete_file_writer.cpp#L137-L155).
   - FE subsequently uses `commit_data.file_path` verbatim in 
`FileMetadata.deleteFileBuilder(...).withPath(...)`: 
[`IcebergWriterHelper.java`](https://github.com/apache/doris/blob/b58b2c53ff56354c692c2dc7634d724d8780838f/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergWriterHelper.java#L316-L337).
   - This explains the observed asymmetry: the data writer has a separate 
`original_write_path`, whereas the delete writer has only `_output_path`. It 
also confirms that standalone DELETE and the delete half of UPDATE/MERGE reach 
the same faulty BE path.
   - The same one-path pattern exists in the format-v3 deletion-vector/Puffin 
lane: `_generate_puffin_file_path()` uses the normalized base and that path is 
assigned directly to `commit_data.file_path`. This is a **code-confirmed 
exposure but was not runtime-reproduced in this report**, so v3 should be 
included in patch validation.
   
   ### Fix direction
   
   Keep separate full paths for every delete artifact:
   
   1. a normalized physical path used only for BE filesystem creation/writes; 
and
   2. a raw/original path returned in `TIcebergCommitData.file_path` and 
committed to Iceberg metadata.
   
   The issue's proposed outcome is correct, but adding new FE plumbing is not 
necessarily required: the current delete sink already receives the raw data 
location as `table_location`. BE can generate one filename and join it to 
`_output_path` for I/O and `_table_location` for the manifest. If an explicit 
`original_output_path` field is preferred for clarity, it needs to be added to 
`TIcebergDeleteSink` and handled compatibly for rolling upgrades. Apply the 
same separation to Puffin/DV files, not only position-delete Parquet/ORC files.
   
   ### Tests and next steps
   
   - Add a focused BE test proving that the filesystem path remains normalized 
while returned commit metadata retains the raw URI.
   - Add ADLS Iceberg v2 regressions for both standalone DELETE and UPDATE: 
verify `$files.file_path` remains `abfss://...` for delete files and verify a 
subsequent table read succeeds.
   - Add a format-v3 deletion-vector case if that configuration is supported by 
the test environment.
   - Backport the fix to `branch-4.1`; the faulty statement is still present at 
the current branch head checked during triage.
   - Until a fix is deployed, avoid row-level DML on affected ADLS tables. For 
an already affected table, rollback to the preceding snapshot remains the safe 
recovery. The 4.1.3 source also contains Doris SQL support for `ALTER TABLE 
<table> EXECUTE rollback_to_snapshot("snapshot_id"="<id>")`; this path appears 
not to resolve the bad delete file and is worth trying before requiring an 
external Iceberg client, followed by `REFRESH TABLE` if needed.
   
   No code changes were made during this triage.
   


-- 
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]

Reply via email to