Skip to content

Commit 5636dcf

Browse files
authored
fix(blob): identify a missing blob file with FileSystem::Exists (#435)
1 parent debf91d commit 5636dcf

17 files changed

Lines changed: 494 additions & 70 deletions

File tree

benchmark/CMakeLists.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ if(PAIMON_BUILD_BENCHMARKS)
5757
${PAIMON_BENCHMARK_STATIC_LINK_LIBS}
5858
test_utils_static
5959
Threads::Threads
60-
${CMAKE_DL_LIBS}
6160
${PAIMON_BENCHMARK_PLATFORM_LINK_LIBS}
6261
${PAIMON_BENCHMARK_LINK_TOOLCHAIN}
6362
EXTRA_INCLUDES

cmake_modules/ThirdpartyToolchain.cmake

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1643,6 +1643,9 @@ macro(build_arrow)
16431643

16441644
# libarrow.a calls dlsym; keep ${CMAKE_DL_LIBS} in the interface so -ldl is placed
16451645
# after libarrow.a on linkers that resolve symbols strictly left-to-right.
1646+
# Every library that uses dl itself (arrow here; also lucene and jindosdk::nextarch)
1647+
# declares it on its own interface the same way; consumers inherit it transitively
1648+
# and must not list ${CMAKE_DL_LIBS} again themselves.
16461649
target_link_libraries(arrow
16471650
INTERFACE zstd
16481651
snappy

include/paimon/defs.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -440,13 +440,14 @@ struct PAIMON_EXPORT Options {
440440
/// path and requires manual configuration by the user. No default value.
441441
static const char BLOB_VIEW_UPSTREAM_WAREHOUSE[];
442442
/// "blob-write-null-on-missing-file" - Whether to write NULL for a descriptor BLOB value when
443-
/// the referenced file does not exist at write time. When false, the write fails when the
444-
/// descriptor is read. Default value is "false".
443+
/// the referenced file does not exist at write time. When false, a missing file is treated
444+
/// like any other fetch failure, following "blob-write-null-on-fetch-failure". Default value
445+
/// is "false".
445446
static const char BLOB_WRITE_NULL_ON_MISSING_FILE[];
446447
/// "blob-write-null-on-fetch-failure" - Whether to write NULL for a descriptor BLOB value when
447448
/// the referenced data cannot be fetched at write time (e.g. invalid descriptor or invalid
448-
/// offset). A missing file is handled by "blob-write-null-on-missing-file". When false, the
449-
/// write fails when the descriptor is read. Default value is "false".
449+
/// offset). A missing file is handled by "blob-write-null-on-missing-file" when that option is
450+
/// enabled. When false, the write fails when the descriptor is read. Default value is "false".
450451
static const char BLOB_WRITE_NULL_ON_FETCH_FAILURE[];
451452
/// "global-index.enabled" - Whether to enable global index for scan. Default value is "true".
452453
static const char GLOBAL_INDEX_ENABLED[];

src/paimon/CMakeLists.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,6 @@ add_paimon_lib(paimon
392392
arrow
393393
tbb
394394
glog
395-
${CMAKE_DL_LIBS}
396395
fmt
397396
roaring_bitmap
398397
xxhash

src/paimon/common/file_index/CMakeLists.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ add_paimon_lib(paimon_file_index
4242
arrow
4343
fmt
4444
xxhash
45-
${CMAKE_DL_LIBS}
4645
Threads::Threads
4746
SHARED_LINK_LIBS
4847
paimon_shared

src/paimon/common/global_index/CMakeLists.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ add_paimon_lib(paimon_global_index
3838
STATIC_LINK_LIBS
3939
arrow
4040
fmt
41-
${CMAKE_DL_LIBS}
4241
Threads::Threads
4342
SHARED_LINK_LIBS
4443
paimon_shared

src/paimon/format/avro/CMakeLists.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ if(PAIMON_ENABLE_AVRO)
3838
fmt
3939
avro
4040
tbb
41-
${CMAKE_DL_LIBS}
4241
Threads::Threads
4342
SHARED_LINK_LIBS
4443
paimon_shared

src/paimon/format/blob/CMakeLists.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ add_paimon_lib(paimon_blob_file_format
2525
STATIC_LINK_LIBS
2626
arrow
2727
fmt
28-
${CMAKE_DL_LIBS}
2928
Threads::Threads
3029
SHARED_LINK_LIBS
3130
paimon_shared

src/paimon/format/blob/blob_format_writer.cpp

Lines changed: 86 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "paimon/common/utils/arrow/status_utils.h"
3131
#include "paimon/common/utils/delta_varint_compressor.h"
3232
#include "paimon/data/blob.h"
33+
#include "paimon/fs/file_system.h"
3334
#include "paimon/io/byte_array_input_stream.h"
3435
#include "paimon/logging.h"
3536

@@ -48,6 +49,8 @@ BlobFormatWriter::BlobFormatWriter(const std::shared_ptr<OutputStream>& out, con
4849
pool_(pool),
4950
write_null_on_missing_file_(write_null_on_missing_file),
5051
write_null_on_fetch_failure_(write_null_on_fetch_failure) {
52+
// Create() has already checked that data_type has exactly one BLOB field.
53+
blob_field_name_ = data_type_->field(0)->name();
5154
metrics_ = std::make_shared<MetricsImpl>();
5255
tmp_buffer_ = Bytes::AllocateBytes(kTmpBufferSize, pool_.get());
5356
magic_number_bytes_ = IntegerToLittleEndian<int32_t>(BlobDefs::kMagicNumber, pool_);
@@ -67,6 +70,9 @@ Result<std::unique_ptr<BlobFormatWriter>> BlobFormatWriter::Create(
6770
if (pool == nullptr) {
6871
return Status::Invalid("blob format writer create failed. pool is nullptr");
6972
}
73+
if (fs == nullptr) {
74+
return Status::Invalid("blob format writer create failed. fs is nullptr");
75+
}
7076
if (data_type->num_fields() != 1) {
7177
return Status::Invalid(
7278
fmt::format("blob data type field number {} is not 1", data_type->num_fields()));
@@ -117,6 +123,10 @@ Status BlobFormatWriter::AddBatch(ArrowArray* batch) {
117123
}
118124

119125
Status BlobFormatWriter::Flush() {
126+
metrics_->SetCounter(BlobMetrics::WRITE_NULL_ON_MISSING_FILE_COUNT,
127+
null_on_missing_file_count_);
128+
metrics_->SetCounter(BlobMetrics::WRITE_NULL_ON_FETCH_FAILURE_COUNT,
129+
null_on_fetch_failure_count_);
120130
return out_->Flush();
121131
}
122132

@@ -140,29 +150,20 @@ Status BlobFormatWriter::Finish() {
140150
Status BlobFormatWriter::WriteBlob(std::string_view blob_data) {
141151
// Open the blob input stream before writing any bytes, so that a failed fetch can be
142152
// converted to a NULL element without leaving partial data in the output stream.
143-
// Dynamically check whether blob_data is a serialized BlobDescriptor (by magic header)
144-
// rather than relying on blob_as_descriptor_ config. This is consistent with Java behavior:
145-
// at write time, the input bytes are auto-detected as descriptor or raw data.
153+
// Whether blob_data is a serialized BlobDescriptor is detected by its magic header rather
154+
// than taken from a blob_as_descriptor option, so each row may hold either form.
146155
std::unique_ptr<InputStream> in;
147156
PAIMON_ASSIGN_OR_RAISE(bool is_descriptor,
148157
BlobDescriptor::IsBlobDescriptor(blob_data.data(), blob_data.size()));
149158
if (is_descriptor) {
150-
Result<std::unique_ptr<InputStream>> opened = OpenDescriptorInputStream(blob_data);
151-
if (!opened.ok()) {
152-
const Status& status = opened.status();
153-
// A missing file is only handled by 'blob-write-null-on-missing-file'; other fetch
154-
// failures are only handled by 'blob-write-null-on-fetch-failure' (aligned with Java).
155-
bool write_null =
156-
status.IsNotExist() ? write_null_on_missing_file_ : write_null_on_fetch_failure_;
157-
if (write_null) {
158-
PAIMON_LOG_WARN(logger_, "Failed to open blob, writing NULL for BLOB field: %s",
159-
status.ToString().c_str());
160-
bin_lengths_.push_back(BlobDefs::kNullBinLength);
161-
return Status::OK();
162-
}
163-
return status;
159+
PAIMON_ASSIGN_OR_RAISE(std::unique_ptr<InputStream> descriptor_in,
160+
OpenDescriptorInputStream(blob_data));
161+
// A null stream means a write-null option already converted the failure.
162+
if (descriptor_in == nullptr) {
163+
bin_lengths_.push_back(BlobDefs::kNullBinLength);
164+
return Status::OK();
164165
}
165-
in = std::move(opened).value();
166+
in = std::move(descriptor_in);
166167
} else {
167168
in = std::make_unique<ByteArrayInputStream>(blob_data.data(), blob_data.size());
168169
}
@@ -206,10 +207,73 @@ Status BlobFormatWriter::WriteBlob(std::string_view blob_data) {
206207
}
207208

208209
Result<std::unique_ptr<InputStream>> BlobFormatWriter::OpenDescriptorInputStream(
209-
std::string_view blob_data) const {
210-
PAIMON_ASSIGN_OR_RAISE(std::unique_ptr<Blob> blob,
211-
Blob::FromDescriptor(blob_data.data(), blob_data.size()));
212-
return blob->NewInputStream(fs_);
210+
std::string_view blob_data) {
211+
// A descriptor that cannot be deserialized is a fetch failure: the referenced data cannot be
212+
// reached. Its URI is inside the unreadable bytes, hence the placeholder; the underlying
213+
// status comes from the byte reader and never mentions blobs, hence the added context.
214+
Result<std::unique_ptr<Blob>> blob_result =
215+
Blob::FromDescriptor(blob_data.data(), blob_data.size());
216+
if (!blob_result.ok()) {
217+
const Status& status = blob_result.status();
218+
return HandleFetchFailure(
219+
"<unknown>", status.WithMessage("invalid blob descriptor: ", status.message()));
220+
}
221+
std::unique_ptr<Blob> blob = std::move(blob_result).value();
222+
223+
// A missing file is identified by FileSystem::Exists rather than by the status of a failed
224+
// open, since file system implementations disagree on which status a missing file maps to.
225+
// The check runs only when `write_null_on_missing_file_` needs the classification; otherwise
226+
// a missing file gets the same treatment as any other failed open.
227+
if (write_null_on_missing_file_) {
228+
Result<bool> exists = fs_->Exists(blob->Uri());
229+
if (exists.ok()) {
230+
if (!exists.value()) {
231+
return HandleMissingFile(blob->Uri());
232+
}
233+
} else if (!write_null_on_fetch_failure_) {
234+
// The check cannot answer whether the file is there; with no fetch-failure handling
235+
// to defer to, fail rather than assume either answer.
236+
const Status& status = exists.status();
237+
return status.WithMessage("failed to check existence of blob file '", blob->Uri(),
238+
"': ", status.message());
239+
}
240+
// A failed check is otherwise deferred to the open below, which can still succeed.
241+
}
242+
243+
Result<std::unique_ptr<InputStream>> opened = blob->NewInputStream(fs_);
244+
if (!opened.ok()) {
245+
// The file can be deleted between the check above and this open. Classifying that from
246+
// `opened.status()` would reintroduce the plugin-specific status codes this writer avoids,
247+
// so ask FileSystem::Exists once more. This narrows the window rather than closing it; a
248+
// check that cannot answer falls through to the open failure.
249+
if (write_null_on_missing_file_) {
250+
Result<bool> exists = fs_->Exists(blob->Uri());
251+
if (exists.ok() && !exists.value()) {
252+
return HandleMissingFile(blob->Uri());
253+
}
254+
}
255+
return HandleFetchFailure(blob->Uri(), opened.status());
256+
}
257+
return std::move(opened).value();
258+
}
259+
260+
std::unique_ptr<InputStream> BlobFormatWriter::HandleMissingFile(const std::string& blob_uri) {
261+
PAIMON_LOG_WARN(logger_, "Blob file %s does not exist, writing NULL for BLOB field %s into %s",
262+
blob_uri.c_str(), blob_field_name_.c_str(), uri_.c_str());
263+
++null_on_missing_file_count_;
264+
return std::unique_ptr<InputStream>();
265+
}
266+
267+
Result<std::unique_ptr<InputStream>> BlobFormatWriter::HandleFetchFailure(
268+
const std::string& blob_uri, const Status& status) {
269+
if (!write_null_on_fetch_failure_) {
270+
return status;
271+
}
272+
PAIMON_LOG_WARN(logger_, "Failed to fetch blob %s, writing NULL for BLOB field %s into %s: %s",
273+
blob_uri.c_str(), blob_field_name_.c_str(), uri_.c_str(),
274+
status.ToString().c_str());
275+
++null_on_fetch_failure_count_;
276+
return std::unique_ptr<InputStream>();
213277
}
214278

215279
Status BlobFormatWriter::WriteBytes(const char* data, int64_t length) {

src/paimon/format/blob/blob_format_writer.h

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,14 +47,27 @@ class OutputStream;
4747

4848
namespace paimon::blob {
4949

50+
class BlobMetrics {
51+
public:
52+
/// Number of rows written as NULL because their referenced file did not exist.
53+
static inline const char WRITE_NULL_ON_MISSING_FILE_COUNT[] =
54+
"blob.write.null-on-missing-file.count";
55+
/// Number of rows written as NULL because their referenced data could not be reached.
56+
static inline const char WRITE_NULL_ON_FETCH_FAILURE_COUNT[] =
57+
"blob.write.null-on-fetch-failure.count";
58+
};
59+
5060
// Blob format:
5161
// https://cwiki.apache.org/confluence/display/PAIMON/PIP-35%3A+Introduce+Blob+to+store+multimodal+data
5262
class BlobFormatWriter : public FormatWriter {
5363
public:
54-
/// When opening a descriptor input fails, `write_null_on_missing_file` converts a
55-
/// missing file (Status::NotExist) to a NULL element and `write_null_on_fetch_failure`
56-
/// converts any other open failure; failures during the streaming copy always fail the
57-
/// write. See Options::BLOB_WRITE_NULL_ON_MISSING_FILE / BLOB_WRITE_NULL_ON_FETCH_FAILURE.
64+
/// `write_null_on_missing_file` converts a descriptor whose referenced file does not exist
65+
/// (as reported by FileSystem::Exists) to a NULL element, and `write_null_on_fetch_failure`
66+
/// converts any other failure to access the referenced data; failures during the streaming
67+
/// copy always fail the write. The existence check runs only when
68+
/// `write_null_on_missing_file` is enabled; otherwise a missing file follows
69+
/// `write_null_on_fetch_failure` like any other failed open.
70+
/// See Options::BLOB_WRITE_NULL_ON_MISSING_FILE / BLOB_WRITE_NULL_ON_FETCH_FAILURE.
5871
static Result<std::unique_ptr<BlobFormatWriter>> Create(
5972
const std::shared_ptr<OutputStream>& out, const std::shared_ptr<arrow::DataType>& data_type,
6073
bool write_null_on_missing_file, bool write_null_on_fetch_failure,
@@ -84,8 +97,19 @@ class BlobFormatWriter : public FormatWriter {
8497
Status WriteBlob(std::string_view blob_data);
8598

8699
/// Deserialize the descriptor and open an input stream on the referenced data.
87-
Result<std::unique_ptr<InputStream>> OpenDescriptorInputStream(
88-
std::string_view blob_data) const;
100+
/// Returns a null stream when the failure is converted to a NULL element by
101+
/// `write_null_on_missing_file_` or `write_null_on_fetch_failure_`.
102+
Result<std::unique_ptr<InputStream>> OpenDescriptorInputStream(std::string_view blob_data);
103+
104+
/// Convert a file that FileSystem::Exists reported as absent to a NULL element: count it and
105+
/// return a null stream. Only reached under `write_null_on_missing_file_`, which callers check.
106+
std::unique_ptr<InputStream> HandleMissingFile(const std::string& blob_uri);
107+
108+
/// Apply `write_null_on_fetch_failure_` to a failure to reach the referenced data: returns
109+
/// `status` when the option is disabled, and a null stream when it converts the failure to a
110+
/// NULL element. `blob_uri` is only used for logging.
111+
Result<std::unique_ptr<InputStream>> HandleFetchFailure(const std::string& blob_uri,
112+
const Status& status);
89113

90114
Status WriteBytes(const char* data, int64_t length);
91115
Status WriteWithCrc32(const char* data, int64_t length);
@@ -100,15 +124,19 @@ class BlobFormatWriter : public FormatWriter {
100124
uint32_t crc32_ = 0;
101125
std::vector<int64_t> bin_lengths_;
102126
std::shared_ptr<OutputStream> out_;
127+
/// Path of the blob file being written, not of any referenced blob.
103128
std::string uri_;
104129
PAIMON_UNIQUE_PTR<Bytes> tmp_buffer_;
105130
PAIMON_UNIQUE_PTR<Bytes> magic_number_bytes_;
106131
std::shared_ptr<arrow::DataType> data_type_;
132+
std::string blob_field_name_;
107133
std::shared_ptr<FileSystem> fs_;
108134
std::shared_ptr<MemoryPool> pool_;
109135
std::shared_ptr<Metrics> metrics_;
110136
bool write_null_on_missing_file_ = false;
111137
bool write_null_on_fetch_failure_ = false;
138+
uint64_t null_on_missing_file_count_ = 0;
139+
uint64_t null_on_fetch_failure_count_ = 0;
112140
std::unique_ptr<Logger> logger_;
113141
};
114142

0 commit comments

Comments
 (0)