924060929 commented on code in PR #66348:
URL: https://github.com/apache/doris/pull/66348#discussion_r3710028088
##########
be/src/exec/sink/viceberg_delete_sink.cpp:
##########
@@ -283,13 +283,17 @@ Status VIcebergDeleteSink::close(Status close_status) {
_delete_file_count);
if (_state != nullptr) {
- for (const auto& commit_data : _commit_data_list) {
- _state->add_iceberg_commit_datas(commit_data);
+ for (auto& commit_data : _commit_data_list) {
+ Status report_status =
_state->add_iceberg_commit_datas(std::move(commit_data));
+ if (!report_status.ok()) {
+ _cleanup_created_files();
+ return report_status;
+ }
}
}
if (!_defer_file_cleanup_until_outer_close) {
- _created_files.clear();
+ _transfer_created_files_to_report_cleanup();
Review Comment:
[P1] Gate the standalone Iceberg delete sink before it creates files when
the coordinator cannot acknowledge external-file reports.
The capability check added in this commit is only in
`VIcebergTableWriter::open()`. A pure Iceberg DELETE uses `ICEBERG_DELETE_SINK`
and opens `VIcebergDeleteSink` directly, so a new BE with an old FE can still
create a position-delete or deletion-vector file and register this cleanup
callback. The old FE accepts `iceberg_commit_datas` and returns `status=OK`,
but its response has no `external_file_commit_data_accepted` field. The new BE
therefore classifies the successful RPC as `REJECTED` rather than `AMBIGUOUS`
and runs this callback, while the old FE can concurrently commit an Iceberg
snapshot that references the deleted file.
Please apply the same `supports_external_file_report_ack` gate in
`VIcebergDeleteSink::open()` before any file can be created, preferably through
a shared helper used by both file-producing sinks, and add an old-coordinator
capability test for the direct delete path.
--
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]
