github-actions[bot] commented on code in PR #66348:
URL: https://github.com/apache/doris/pull/66348#discussion_r3702544448
##########
fe/fe-filesystem/fe-filesystem-azure/src/main/java/org/apache/doris/filesystem/azure/AzureObjStorage.java:
##########
@@ -244,15 +276,37 @@ public void completeMultipartUpload(String remotePath,
String uploadId,
List<UploadPartResult> parts) throws IOException {
try {
AzureUri uri = AzureUri.parse(remotePath);
- BlockBlobClient blockBlobClient =
getClient().getBlobContainerClient(uri.container())
- .getBlobClient(uri.key()).getBlockBlobClient();
+ BlobContainerClient containerClient =
getClient().getBlobContainerClient(uri.container());
List<String> blockIds = new ArrayList<>();
List<UploadPartResult> sorted = new ArrayList<>(parts);
sorted.sort((a, b) -> Integer.compare(a.partNumber(),
b.partNumber()));
+ boolean exactBlockIds = !sorted.isEmpty() && sorted.stream()
+ .allMatch(part -> part.etag() != null &&
!part.etag().isEmpty());
for (UploadPartResult part : sorted) {
- blockIds.add(toBlockId(part.partNumber()));
+ // Missing IDs identify an older BE upload, whose blocks use
the legacy namespace.
Review Comment:
[P1] Reject old-BE Azure writes before publishing Hive metadata
The target-branch Azure BE returns success from multipart initiation without
an upload ID. Because Hive defers completion, its blocks stay uncommitted, and
`VHivePartitionWriter` omits the entire pending-upload record when that ID is
empty, so this fallback is never invoked. The FE can then commit the reported
file name while the blob is still invisible; if an old-BE update is merged
first, `HiveWriteUtils` also drops later new-BE pending tokens. Please require
one Azure completion record per reported file or capability-gate deferred Azure
writes during rolling upgrade, and test an actual base-BE update with no
pending list plus the mixed merge order, rather than a fabricated ID with empty
ETags. Azure publishes staged blocks only through [Put Block
List](https://learn.microsoft.com/en-us/rest/api/storageservices/put-block-list).
##########
be/src/exec/operator/spill_iceberg_table_sink_operator.cpp:
##########
@@ -55,47 +55,62 @@ size_t
SpillIcebergTableSinkLocalState::get_reserve_mem_size(RuntimeState* state
if (!_writer) {
return 0;
}
- auto current_writer = _writer->current_writer();
- auto* sort_writer =
dynamic_cast<VIcebergSortWriter*>(current_writer.get());
- if (!sort_writer) {
- return 0;
+ std::vector<IcebergSorterReserveMemory> per_partition_reservations;
+ auto active_writers = _writer->active_writers();
+ per_partition_reservations.reserve(active_writers->size());
+ for (const auto& writer : *active_writers) {
+ if (auto* sort_writer =
dynamic_cast<VIcebergSortWriter*>(writer.get())) {
+ auto reservation =
sort_writer->get_reserve_mem_size_components(state, eos);
Review Comment:
[P1] Account for sorter state already queued ahead of this block
This estimate samples the async writer's currently published sorter state,
but its queue accepts three blocks. If the consumer is stalled, all three
producer-side calls can see the same pre-queue rows/capacity; after earlier
queued blocks append, a later token can cross the 85% growth or sort threshold
without reserving that allocation. The earlier tokens have already been shrunk
by then, so transferring each token across threads still lets this sink exceed
the hard memory limit instead of spilling. Please account for projected sorter
state from blocks already queued ahead (or allow only one outstanding block),
and add a deterministic multi-queued boundary test.
--
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]
