mapleFU commented on code in PR #41232:
URL: https://github.com/apache/arrow/pull/41232#discussion_r1567826495
##########
cpp/src/arrow/filesystem/gcsfs.cc:
##########
@@ -200,6 +201,16 @@ class GcsOutputStream : public arrow::io::OutputStream {
return internal::ToArrowStatus(stream_.last_status());
}
+ Future<> CloseAsync() override {
+ if (closed_) return Status::OK();
+
+ auto self = std::dynamic_pointer_cast<GcsOutputStream>(shared_from_this());
+ auto deferred = [self = std::move(self)]() -> Status { return
self->Close(); };
Review Comment:
I've check the s3fs, maybe it also doesn't prevent from
`FinishPartUploadAfterFlush()` being called multiple times. It's possible to
make gcs safe by holding a future here.
For gcs, maybe the code below is thread safe if we ensure `Close/CloseAsync`
wouldn't be called concurrently
```
Status Close() override {
if (closed_) {
return Status::OK();
}
return CloseAsync().status();
}
Future<> CloseAsync() override {
if (closed_) return Status::OK();
if (this->close_future_.is_valid()) {
return close_future_;
}
auto self =
std::dynamic_pointer_cast<GcsOutputStream>(shared_from_this());
auto deferred = [self = std::move(self)]() -> Status {
self->stream_.Close();
self->closed_ = true;
return internal::ToArrowStatus(self->stream_.last_status());
};
ARROW_ASSIGN_OR_RAISE(close_future_,
io::internal::SubmitIO(io_context_,
std::move(deferred)));
return close_future_;
}
```
--
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]
