Gabriel39 commented on code in PR #67328:
URL: https://github.com/apache/doris/pull/67328#discussion_r3911822405
##########
be/src/service/internal_service.cpp:
##########
@@ -770,41 +819,156 @@ void
PInternalService::outfile_write_success(google::protobuf::RpcController* co
return;
}
- auto&& res = FileFactory::create_file_writer(file_type_res.value(),
ExecEnv::GetInstance(),
-
file_options.broker_addresses,
-
file_options.broker_properties, file_name,
- {
- .write_file_cache
= false,
- .sync_file_data =
false,
- });
- using T = std::decay_t<decltype(res)>;
- if (!res.has_value()) [[unlikely]] {
- st = std::forward<T>(res).error();
+ io::FSPropertiesRef properties(file_type_res.value());
+ properties.broker_addresses = &file_options.broker_addresses;
+ properties.properties = &file_options.broker_properties;
+ io::FileDescription file_description;
+ file_description.path = file_name;
+ auto fs_res = FileFactory::create_fs(properties, file_description);
+ if (!fs_res.has_value()) [[unlikely]] {
+ st = std::move(fs_res).error();
+ st.to_protobuf(result->mutable_status());
+ return;
+ }
+ auto file_system = std::move(fs_res).value();
+
+ if (request->operation() == OUTFILE_MARKER_DELETE) {
+ // Delete only a path created by this token. A blind rollback
could otherwise remove a
+ // pre-existing user file when CREATE failed before acquiring
ownership.
+ if (owned_marker_path.empty()) {
+ Status::OK().to_protobuf(result->mutable_status());
+ return;
+ }
+ st = file_system->delete_file(owned_marker_path);
+ if (st.ok() || st.is<ErrorCode::NOT_FOUND>()) {
+ std::lock_guard marker_guard(outfile_marker_lock);
+ auto state_it = outfile_marker_states.find(marker_token);
+ if (state_it != outfile_marker_states.end() &&
+ state_it->second.owned_path == owned_marker_path) {
+ state_it->second.owned_path.clear();
+ state_it->second.updated_at =
std::chrono::steady_clock::now();
+ }
+ st = Status::OK();
+ }
+ st.to_protobuf(result->mutable_status());
+ return;
+ }
+ if (request->operation() != OUTFILE_MARKER_CREATE) {
+ Status::InvalidArgument("unknown OUTFILE success marker operation")
+ .to_protobuf(result->mutable_status());
+ return;
+ }
+
+ // Never claim an existing marker path; rollback is restricted to
token-owned paths below.
+ bool exists = true;
+ st = file_system->exists(file_name, &exists);
Review Comment:
Fixed in d37cfa0cd7. Marker-operation locking, ownership state, tombstones,
and the cross-storage existence check are now enabled only when
enable_atomic_outfile is true. Legacy remote requests with the field absent or
explicitly false bypass the new policy, while LOCAL retains its existing
rejection behavior. The compatibility test covers field-absent and
explicit-false requests plus atomic S3, HDFS, and Broker requests.
--
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]
