Skip to content

Commit

Permalink
[fix](split) remove retry when fetch split batch failed (#37636)
Browse files Browse the repository at this point in the history
## Proposed changes

We need to remove the retry logic for failed to fetch split batch.
Originally, this was implemented to handle cases where the cached client
connection might have been lost and needed to be reestablished. However,
this retry mechanism can lead to data loss. For instance, if a batch of
data has already been sent, retrying can cause this batch to be lost
without the receiver being aware of it.
  • Loading branch information
AshinGau authored Jul 12, 2024
1 parent 9b7159e commit 83a734e
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 9 deletions.
10 changes: 2 additions & 8 deletions be/src/vec/exec/scan/split_source_connector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,8 @@ Status RemoteSplitSourceConnector::get_next(bool* has_next, TFileRangeDesc* rang
TFetchSplitBatchResult result;
try {
coord->fetchSplitBatch(result, request);
} catch (std::exception& e1) {
LOG(WARNING) << "Failed to get batch of split source: {}, try to reopen" << e1.what();
RETURN_IF_ERROR(coord.reopen());
try {
coord->fetchSplitBatch(result, request);
} catch (std::exception& e2) {
return Status::IOError("Failed to get batch of split source: {}", e2.what());
}
} catch (std::exception& e) {
return Status::IOError<false>("Failed to get batch of split source: {}", e.what());
}
_last_batch = result.splits.empty();
_scan_ranges = result.splits;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,12 @@ public void removeSplitSource(long uniqueId) {
}

public SplitSource getSplitSource(long uniqueId) {
return splits.get(uniqueId).get();
WeakReference<SplitSource> ref = splits.get(uniqueId);
if (ref == null) {
return null;
} else {
return ref.get();
}
}

@Override
Expand Down

0 comments on commit 83a734e

Please sign in to comment.