Skip to content

Commit 1320a52

Browse files
amitkduttafacebook-github-bot
authored andcommitted
misc: Throw generic spill error in validateSpillBytesSize (facebookincubator#12665)
Summary: Throwing spilling exception to properly catagorize spilling issues. Eventually kGenericSpillFailure will map to GENERIC_SPILL_ERROR in [prestodb](https://github.com/prestodb/presto/blob/d87438d66825629f164bc9aa69c8f33d95ef6cca/presto-spi/src/main/java/com/facebook/presto/spi/StandardErrorCode.java#L113) Reviewed By: xiaoxmeng, tanjialiang Differential Revision: D71249799
1 parent 407aa54 commit 1320a52

File tree

3 files changed

+18
-1
lines changed

3 files changed

+18
-1
lines changed

velox/common/base/SpillConfig.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,15 @@ namespace facebook::velox::common {
3434
"{}", \
3535
errorMessage);
3636

37+
#define VELOX_GENERIC_SPILL_FAILURE(errorMessage) \
38+
_VELOX_THROW( \
39+
::facebook::velox::VeloxRuntimeError, \
40+
::facebook::velox::error_source::kErrorSourceRuntime.c_str(), \
41+
::facebook::velox::error_code::kGenericSpillFailure.c_str(), \
42+
/* isRetriable */ true, \
43+
"{}", \
44+
errorMessage);
45+
3746
/// Defining type for a callback function that returns the spill directory path.
3847
/// Implementations can use it to ensure the path exists before returning.
3948
using GetSpillDirectoryPathCB = std::function<std::string_view()>;

velox/common/base/VeloxException.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,9 @@ inline constexpr auto kNoCacheSpace = "NO_CACHE_SPACE"_fs;
109109
/// An error raised when spill bytes exceeds limits.
110110
inline constexpr auto kSpillLimitExceeded = "SPILL_LIMIT_EXCEEDED"_fs;
111111

112+
/// An error raised to indicate any general failure happened during spilling.
113+
inline constexpr auto kGenericSpillFailure = "GENERIC_SPILL_FAILURE"_fs;
114+
112115
/// An error raised when trace bytes exceeds limits.
113116
inline constexpr auto kTraceLimitExceeded = "TRACE_LIMIT_EXCEEDED"_fs;
114117

velox/exec/Spill.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,12 @@ void SpillState::setPartitionSpilled(uint32_t partition) {
133133
void SpillState::validateSpillBytesSize(uint64_t bytes) {
134134
static constexpr uint64_t kMaxSpillBytesPerWrite =
135135
std::numeric_limits<int32_t>::max();
136-
VELOX_CHECK_LT(bytes, kMaxSpillBytesPerWrite, "Spill bytes will overflow.");
136+
if (bytes >= kMaxSpillBytesPerWrite) {
137+
VELOX_GENERIC_SPILL_FAILURE(fmt::format(
138+
"Spill bytes will overflow. Bytes {}, kMaxSpillBytesPerWrite: {}",
139+
bytes,
140+
kMaxSpillBytesPerWrite));
141+
}
137142
}
138143

139144
void SpillState::updateSpilledInputBytes(uint64_t bytes) {

0 commit comments

Comments
 (0)