You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "ByteYue (via GitHub)" <gi...@apache.org> on 2023/03/08 15:07:03 UTC
[GitHub] [doris] ByteYue opened a new pull request, #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
ByteYue opened a new pull request, #17585:
URL: https://github.com/apache/doris/pull/17585
# Proposed changes
Issue Number: close #xxx
Formerly S3FileWriter has to write each buffer with 5MB or more then upload one part, after all these works are done it could then process the incoming data, it's blocking and inefficient. This pr brings one bufferpool where the data could write into memory buffer immediately if has free buffer and then it would be uploaded into the S3.
This pr doesn't provide the ability to elegantly support cases where there is no free buffer, i'll leave it as one future work.
## Problem summary
Describe your changes.
## Checklist(Required)
* [ ] Does it affect the original behavior
* [ ] Has unit tests been added
* [ ] Has document been added or modified
* [ ] Does it need to update dependencies
* [ ] Is this PR support rollback (If NO, please explain WHY)
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #17585:
URL: https://github.com/apache/doris/pull/17585#issuecomment-1517953575
PR approved by at least one committer and no changes requested.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] ByteYue commented on pull request #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
Posted by "ByteYue (via GitHub)" <gi...@apache.org>.
ByteYue commented on PR #17585:
URL: https://github.com/apache/doris/pull/17585#issuecomment-1495350409
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] ByteYue commented on pull request #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
Posted by "ByteYue (via GitHub)" <gi...@apache.org>.
ByteYue commented on PR #17585:
URL: https://github.com/apache/doris/pull/17585#issuecomment-1508010919
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] ByteYue commented on pull request #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
Posted by "ByteYue (via GitHub)" <gi...@apache.org>.
ByteYue commented on PR #17585:
URL: https://github.com/apache/doris/pull/17585#issuecomment-1510797110
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #17585:
URL: https://github.com/apache/doris/pull/17585#issuecomment-1495422509
clang-tidy review says "All clean, LGTM! :+1:"
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] ByteYue commented on pull request #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
Posted by "ByteYue (via GitHub)" <gi...@apache.org>.
ByteYue commented on PR #17585:
URL: https://github.com/apache/doris/pull/17585#issuecomment-1517998596
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] ByteYue commented on pull request #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
Posted by "ByteYue (via GitHub)" <gi...@apache.org>.
ByteYue commented on PR #17585:
URL: https://github.com/apache/doris/pull/17585#issuecomment-1495419294
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #17585:
URL: https://github.com/apache/doris/pull/17585#issuecomment-1510801358
clang-tidy review says "All clean, LGTM! :+1:"
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #17585:
URL: https://github.com/apache/doris/pull/17585#issuecomment-1512408922
clang-tidy review says "All clean, LGTM! :+1:"
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] hello-stephen commented on pull request #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #17585:
URL: https://github.com/apache/doris/pull/17585#issuecomment-1495425668
TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 33.42 seconds
stream load tsv: 443 seconds loaded 74807831229 Bytes, about 161 MB/s
stream load json: 25 seconds loaded 2358488459 Bytes, about 89 MB/s
stream load orc: 73 seconds loaded 1101869774 Bytes, about 14 MB/s
stream load parquet: 30 seconds loaded 861443392 Bytes, about 27 MB/s
https://doris-community-test-1308700295.cos.ap-hongkong.myqcloud.com/tmp/20230404063535_clickbench_pr_124493.html
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] morningman merged pull request #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
Posted by "morningman (via GitHub)" <gi...@apache.org>.
morningman merged PR #17585:
URL: https://github.com/apache/doris/pull/17585
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] ByteYue commented on pull request #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
Posted by "ByteYue (via GitHub)" <gi...@apache.org>.
ByteYue commented on PR #17585:
URL: https://github.com/apache/doris/pull/17585#issuecomment-1518545681
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] ByteYue commented on pull request #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
Posted by "ByteYue (via GitHub)" <gi...@apache.org>.
ByteYue commented on PR #17585:
URL: https://github.com/apache/doris/pull/17585#issuecomment-1512406956
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on a diff in pull request #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #17585:
URL: https://github.com/apache/doris/pull/17585#discussion_r1173847482
##########
be/test/io/fs/remote_file_system_test.cpp:
##########
@@ -392,6 +394,12 @@ TEST_F(RemoteFileSystemTest, TestHdfsFileSystem) {
}
TEST_F(RemoteFileSystemTest, TestS3FileSystem) {
+ std::unique_ptr<ThreadPool> _pool;
+ ThreadPoolBuilder("BufferedReaderPrefetchThreadPool")
+ .set_min_threads(5)
+ .set_max_threads(10)
+ .build(&_pool);
+ ExecEnv::GetInstance()->_buffered_reader_prefetch_thread_pool = std::move(_pool);
Review Comment:
warning: '_buffered_reader_prefetch_thread_pool' is a private member of 'doris::ExecEnv' [clang-diagnostic-error]
```cpp
ExecEnv::GetInstance()->_buffered_reader_prefetch_thread_pool = std::move(_pool);
^
```
**be/src/runtime/exec_env.h:218:** declared private here
```cpp
std::unique_ptr<ThreadPool> _buffered_reader_prefetch_thread_pool;
^
```
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on a diff in pull request #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #17585:
URL: https://github.com/apache/doris/pull/17585#discussion_r1173823759
##########
be/test/io/fs/remote_file_system_test.cpp:
##########
@@ -392,6 +394,12 @@ TEST_F(RemoteFileSystemTest, TestHdfsFileSystem) {
}
TEST_F(RemoteFileSystemTest, TestS3FileSystem) {
+ std::unique_ptr<ThreadPool> _pool;
+ ThreadPoolBuilder("BufferedReaderPrefetchThreadPool")
+ .set_min_threads(5)
+ .set_max_threads(10)
+ .build(&_pool);
+ ExecEnv::GetInstance()->_buffered_reader_prefetch_thread_pool = std::move(_pool);
Review Comment:
warning: '_buffered_reader_prefetch_thread_pool' is a private member of 'doris::ExecEnv' [clang-diagnostic-error]
```cpp
ExecEnv::GetInstance()->_buffered_reader_prefetch_thread_pool = std::move(_pool);
^
```
**be/src/runtime/exec_env.h:225:** declared private here
```cpp
std::unique_ptr<ThreadPool> _buffered_reader_prefetch_thread_pool;
^
```
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #17585:
URL: https://github.com/apache/doris/pull/17585#issuecomment-1508010857
clang-tidy review says "All clean, LGTM! :+1:"
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #17585:
URL: https://github.com/apache/doris/pull/17585#issuecomment-1518669355
PR approved by at least one committer and no changes requested.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on a diff in pull request #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #17585:
URL: https://github.com/apache/doris/pull/17585#discussion_r1129593883
##########
be/src/io/fs/s3_file_writer.h:
##########
@@ -33,13 +33,17 @@
namespace doris {
namespace io {
+class S3FileBuffer;
class S3FileWriter final : public FileWriter {
public:
- S3FileWriter(Path path, std::shared_ptr<Aws::S3::S3Client> client, const S3Conf& s3_conf);
+ S3FileWriter(Path path, std::string key, std::string bucket, std::shared_ptr<Aws::S3::S3Client>,
+ std::shared_ptr<S3FileSystem> fs, IOContext* state);
~S3FileWriter() override;
- Status close() override;
+ Status open() override;
+
+ Status close(bool sync = true) override;
Review Comment:
warning: non-virtual member function marked 'override' hides virtual member function [clang-diagnostic-error]
```cpp
Status close(bool sync = true) override;
^
```
**be/src/io/fs/file_writer.h:38:** hidden overloaded virtual function 'doris::io::FileWriter::close' declared here: different number of parameters (0 vs 1)
```cpp
virtual Status close() = 0;
^
```
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #17585:
URL: https://github.com/apache/doris/pull/17585#issuecomment-1495868663
clang-tidy review says "All clean, LGTM! :+1:"
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on a diff in pull request #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #17585:
URL: https://github.com/apache/doris/pull/17585#discussion_r1129593866
##########
be/src/io/fs/s3_file_writer.h:
##########
@@ -33,13 +33,17 @@ class S3Client;
namespace doris {
namespace io {
+class S3FileBuffer;
class S3FileWriter final : public FileWriter {
Review Comment:
warning: abstract class is marked 'final' [clang-diagnostic-abstract-final-class]
```cpp
class S3FileWriter final : public FileWriter {
^
```
**be/src/io/fs/file_writer.h:38:** unimplemented pure virtual method 'close' in 'S3FileWriter'
```cpp
virtual Status close() = 0;
^
```
##########
be/src/io/fs/s3_file_writer.h:
##########
@@ -54,31 +58,78 @@
size_t bytes_appended() const override { return _bytes_appended; }
FileSystemSPtr fs() const override { return _fs; }
+ int64_t upload_cost_ms() const { return *_upload_cost_ms; }
-private:
- std::shared_ptr<S3FileSystem> _fs;
+ const Path& path() const override { return _path; }
Review Comment:
warning: only virtual member functions can be marked 'override' [clang-diagnostic-error]
```suggestion
const Path& path() const { return _path; }
```
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on a diff in pull request #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #17585:
URL: https://github.com/apache/doris/pull/17585#discussion_r1156699107
##########
be/src/io/fs/s3_file_write_bufferpool.h:
##########
@@ -0,0 +1,141 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include <condition_variable>
+#include <cstdint>
+#include <fstream>
+#include <functional>
+#include <list>
+#include <memory>
+#include <mutex>
+
+#include "io/fs/s3_common.h"
+#include "common/config.h"
+#include "common/status.h"
+#include "util/slice.h"
+
+namespace doris {
+namespace io {
+
+// TODO(AlexYue): 1. support write into cache 2. unify write buffer and read buffer
+struct S3FileBuffer : public std::enable_shared_from_this<S3FileBuffer> {
Review Comment:
warning: 'S3FileBuffer' defined as a struct here but previously declared as a class; this is valid, but may result in linker errors under the Microsoft C++ ABI [clang-diagnostic-mismatched-tags]
```cpp
struct S3FileBuffer : public std::enable_shared_from_this<S3FileBuffer> {
^
```
**be/src/io/fs/s3_file_writer.h:36:** did you mean struct here?
```cpp
class S3FileBuffer;
^
```
##########
be/src/io/fs/s3_file_writer.h:
##########
@@ -33,38 +34,94 @@ class S3Client;
namespace doris {
namespace io {
+class S3FileBuffer;
class S3FileWriter final : public FileWriter {
public:
- S3FileWriter(Path path, std::shared_ptr<Aws::S3::S3Client> client, const S3Conf& s3_conf,
+ S3FileWriter(Path&& path, std::string&& key, std::shared_ptr<Aws::S3::S3Client> client, const S3Conf& s3_conf,
FileSystemSPtr fs);
~S3FileWriter() override;
+ Status open() ;
+
Status close() override;
+
Status abort() override;
Status appendv(const Slice* data, size_t data_cnt) override;
Status finalize() override;
Status write_at(size_t offset, const Slice& data) override {
return Status::NotSupported("not support");
}
-private:
- Status _close();
- Status _open();
- Status _upload_part();
- void _reset_stream();
+ size_t bytes_appended() const { return _bytes_appended; }
+
+ int64_t upload_cost_ms() const { return *_upload_cost_ms; }
private:
+ class WaitGroup {
+ public:
+ WaitGroup() = default;
+
+ ~WaitGroup() = default;
+
+ WaitGroup(const WaitGroup&) = delete;
+ WaitGroup(WaitGroup&&) = delete;
+ void operator=(const WaitGroup&) = delete;
+ void operator=(WaitGroup&&) = delete;
+ // add one counter indicating one more concurrent worker
+ void add(int count = 1) { _count += count; }
+
+ // decrese count if one concurrent worker finished it's work
+ void done() {
+ _count--;
+ if (_count.load() <= 0) {
+ _cv.notify_all();
+ }
+ }
+
+ // wait for all concurrent workers finish their work
+ // would return if timeout, default timeout would be 5min
+ void wait(int64_t timeout_seconds = 300) {
+ if (_count.load() <= 0) {
+ return;
+ }
+ std::unique_lock<std::mutex> lck {_lock};
+ _cv.wait_for(lck, std::chrono::seconds(timeout_seconds),
+ [this]() { return _count.load() <= 0; });
+ }
+
+ private:
+ std::mutex _lock;
+ std::condition_variable _cv;
+ std::atomic_int64_t _count {0};
+ };
+ Status _complete();
+ void _upload_one_part(int64_t part_num, S3FileBuffer& buf);
+
+ std::string _bucket;
+ std::string _key;
+ bool _closed = true;
+ bool _opened = false;
+
+ std::shared_ptr<int64_t> _upload_cost_ms;
+
std::shared_ptr<Aws::S3::S3Client> _client;
- S3Conf _s3_conf;
std::string _upload_id;
- bool _is_open = false;
- bool _closed = false;
+ size_t _bytes_appended {0};
+ size_t _index_offset {0};
Review Comment:
warning: private field '_index_offset' is not used [clang-diagnostic-unused-private-field]
```cpp
size_t _index_offset {0};
^
```
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #17585:
URL: https://github.com/apache/doris/pull/17585#issuecomment-1495351944
clang-tidy review says "All clean, LGTM! :+1:"
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] morningman commented on a diff in pull request #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
Posted by "morningman (via GitHub)" <gi...@apache.org>.
morningman commented on code in PR #17585:
URL: https://github.com/apache/doris/pull/17585#discussion_r1173326302
##########
be/src/common/config.h:
##########
@@ -941,6 +941,9 @@ CONF_mInt64(max_tablet_io_errors, "-1");
// Page size of row column, default 4KB
CONF_mInt64(row_column_page_size, "4096");
+// it must be larger than or equal to 5MB
+CONF_mInt32(s3_write_buffer_size, "5242880");
+CONF_mInt32(s3_write_buffer_whole_size, "524288000");
Review Comment:
what is `s3_write_buffer_whole_size` means? please add comment in code
##########
be/src/io/fs/s3_file_system.cpp:
##########
@@ -98,7 +98,9 @@ Status S3FileSystem::connect_impl() {
Status S3FileSystem::create_file_impl(const Path& file, FileWriterPtr* writer) {
GET_KEY(key, file);
- *writer = std::make_unique<S3FileWriter>(key, get_client(), _s3_conf, getSPtr());
+ auto fs_path = Path(_s3_conf.endpoint) / _s3_conf.bucket / key;
Review Comment:
Why changing this behavior?
##########
be/src/io/fs/s3_file_writer.cpp:
##########
@@ -17,221 +17,232 @@
#include "io/fs/s3_file_writer.h"
-#include <aws/core/Aws.h>
#include <aws/core/utils/HashingUtils.h>
+#include <aws/core/utils/memory/stl/AWSStreamFwd.h>
+#include <aws/core/utils/memory/stl/AWSStringStream.h>
#include <aws/s3/S3Client.h>
#include <aws/s3/model/AbortMultipartUploadRequest.h>
#include <aws/s3/model/CompleteMultipartUploadRequest.h>
+#include <aws/s3/model/CompletedPart.h>
#include <aws/s3/model/CreateMultipartUploadRequest.h>
-#include <aws/s3/model/DeleteObjectRequest.h>
-#include <aws/s3/model/DeleteObjectsRequest.h>
-#include <aws/s3/model/GetObjectRequest.h>
#include <aws/s3/model/UploadPartRequest.h>
-#include <fmt/core.h>
-#include <sys/uio.h>
-#include <cerrno>
+#include <atomic>
+#include <memory>
+#include <utility>
-#include "common/compiler_util.h"
+#include "common/logging.h"
#include "common/status.h"
-#include "gutil/macros.h"
+#include "io/fs/file_system.h"
#include "io/fs/file_writer.h"
-#include "io/fs/path.h"
#include "io/fs/s3_file_system.h"
-#include "util/doris_metrics.h"
-
-using Aws::S3::Model::AbortMultipartUploadRequest;
-using Aws::S3::Model::CompletedPart;
-using Aws::S3::Model::CompletedMultipartUpload;
-using Aws::S3::Model::CompleteMultipartUploadRequest;
-using Aws::S3::Model::CreateMultipartUploadRequest;
-using Aws::S3::Model::DeleteObjectRequest;
-using Aws::S3::Model::UploadPartRequest;
-using Aws::S3::Model::UploadPartOutcome;
+#include "io/fs/s3_file_write_bufferpool.h"
+#include "util/runtime_profile.h"
+#include "util/s3_util.h"
namespace doris {
namespace io {
+using namespace Aws::S3::Model;
+using Aws::S3::S3Client;
-// max size of each part when uploading: 5MB
-static const int MAX_SIZE_EACH_PART = 5 * 1024 * 1024;
-static const char* STREAM_TAG = "S3FileWriter";
-
-S3FileWriter::S3FileWriter(Path path, std::shared_ptr<Aws::S3::S3Client> client,
+S3FileWriter::S3FileWriter(Path&& path, std::string&& key, std::shared_ptr<S3Client> client,
const S3Conf& s3_conf, FileSystemSPtr fs)
- : FileWriter(std::move(path), fs), _client(client), _s3_conf(s3_conf) {
- DorisMetrics::instance()->s3_file_open_writing->increment(1);
- DorisMetrics::instance()->s3_file_writer_total->increment(1);
-}
+ : FileWriter(std::move(path), std::move(fs)),
+ _bucket(s3_conf.bucket),
+ _key(std::move(key)),
+ _client(std::move(client)) {}
S3FileWriter::~S3FileWriter() {
- if (_opened) {
- close();
+ if (!_closed) {
+ abort();
}
CHECK(!_opened || _closed) << "open: " << _opened << ", closed: " << _closed;
}
-Status S3FileWriter::close() {
- return _close();
+Status S3FileWriter::open() {
+ VLOG_DEBUG << "S3FileWriter::open, path: " << _path.native();
+ CreateMultipartUploadRequest create_request;
+ create_request.WithBucket(_bucket).WithKey(_key);
+ create_request.SetContentType("text/plain");
+
+ auto outcome = _client->CreateMultipartUpload(create_request);
+
+ if (outcome.IsSuccess()) {
+ _upload_id = outcome.GetResult().GetUploadId();
+ _closed = false;
+ _opened = true;
+ return Status::OK();
+ }
+ return Status::IOError("failed to create multipart upload(bucket={}, key={}, upload_id={}): {}",
+ _bucket, _path.native(), _upload_id, outcome.GetError().GetMessage());
}
Status S3FileWriter::abort() {
+ _failed = true;
+ if (_closed || !_opened) {
+ return Status::OK();
+ }
+ VLOG_DEBUG << "S3FileWriter::abort, path: " << _path.native();
+ _closed = true;
+ _wait.wait();
Review Comment:
Check the return value of `wait()`
##########
be/src/io/fs/s3_file_writer.h:
##########
@@ -33,38 +34,93 @@ class S3Client;
namespace doris {
namespace io {
+struct S3FileBuffer;
class S3FileWriter final : public FileWriter {
public:
- S3FileWriter(Path path, std::shared_ptr<Aws::S3::S3Client> client, const S3Conf& s3_conf,
- FileSystemSPtr fs);
+ S3FileWriter(Path&& path, std::string&& key, std::shared_ptr<Aws::S3::S3Client> client,
+ const S3Conf& s3_conf, FileSystemSPtr fs);
~S3FileWriter() override;
+ Status open();
+
Status close() override;
+
Status abort() override;
Status appendv(const Slice* data, size_t data_cnt) override;
Status finalize() override;
Status write_at(size_t offset, const Slice& data) override {
return Status::NotSupported("not support");
}
-private:
- Status _close();
- Status _open();
- Status _upload_part();
- void _reset_stream();
+ size_t bytes_appended() const { return _bytes_appended; }
+
+ int64_t upload_cost_ms() const { return *_upload_cost_ms; }
private:
+ class WaitGroup {
+ public:
+ WaitGroup() = default;
+
+ ~WaitGroup() = default;
+
+ WaitGroup(const WaitGroup&) = delete;
+ WaitGroup(WaitGroup&&) = delete;
+ void operator=(const WaitGroup&) = delete;
+ void operator=(WaitGroup&&) = delete;
+ // add one counter indicating one more concurrent worker
+ void add(int count = 1) { _count += count; }
+
+ // decrese count if one concurrent worker finished it's work
Review Comment:
```suggestion
// decrease count if one concurrent worker finished it's work
```
##########
be/src/io/fs/s3_file_writer.h:
##########
@@ -33,38 +34,93 @@ class S3Client;
namespace doris {
namespace io {
+struct S3FileBuffer;
class S3FileWriter final : public FileWriter {
public:
- S3FileWriter(Path path, std::shared_ptr<Aws::S3::S3Client> client, const S3Conf& s3_conf,
- FileSystemSPtr fs);
+ S3FileWriter(Path&& path, std::string&& key, std::shared_ptr<Aws::S3::S3Client> client,
+ const S3Conf& s3_conf, FileSystemSPtr fs);
~S3FileWriter() override;
+ Status open();
+
Status close() override;
+
Status abort() override;
Status appendv(const Slice* data, size_t data_cnt) override;
Status finalize() override;
Status write_at(size_t offset, const Slice& data) override {
return Status::NotSupported("not support");
}
-private:
- Status _close();
- Status _open();
- Status _upload_part();
- void _reset_stream();
+ size_t bytes_appended() const { return _bytes_appended; }
+
+ int64_t upload_cost_ms() const { return *_upload_cost_ms; }
private:
+ class WaitGroup {
+ public:
+ WaitGroup() = default;
+
+ ~WaitGroup() = default;
+
+ WaitGroup(const WaitGroup&) = delete;
+ WaitGroup(WaitGroup&&) = delete;
+ void operator=(const WaitGroup&) = delete;
+ void operator=(WaitGroup&&) = delete;
+ // add one counter indicating one more concurrent worker
+ void add(int count = 1) { _count += count; }
+
+ // decrese count if one concurrent worker finished it's work
+ void done() {
+ _count--;
+ if (_count.load() <= 0) {
+ _cv.notify_all();
+ }
+ }
+
+ // wait for all concurrent workers finish their work
+ // would return if timeout, default timeout would be 5min
+ void wait(int64_t timeout_seconds = 300) {
Review Comment:
need a return value? Or we don't know it is finished or not
##########
be/src/io/fs/s3_file_writer.cpp:
##########
@@ -17,221 +17,232 @@
#include "io/fs/s3_file_writer.h"
-#include <aws/core/Aws.h>
#include <aws/core/utils/HashingUtils.h>
+#include <aws/core/utils/memory/stl/AWSStreamFwd.h>
+#include <aws/core/utils/memory/stl/AWSStringStream.h>
#include <aws/s3/S3Client.h>
#include <aws/s3/model/AbortMultipartUploadRequest.h>
#include <aws/s3/model/CompleteMultipartUploadRequest.h>
+#include <aws/s3/model/CompletedPart.h>
#include <aws/s3/model/CreateMultipartUploadRequest.h>
-#include <aws/s3/model/DeleteObjectRequest.h>
-#include <aws/s3/model/DeleteObjectsRequest.h>
-#include <aws/s3/model/GetObjectRequest.h>
#include <aws/s3/model/UploadPartRequest.h>
-#include <fmt/core.h>
-#include <sys/uio.h>
-#include <cerrno>
+#include <atomic>
+#include <memory>
+#include <utility>
-#include "common/compiler_util.h"
+#include "common/logging.h"
#include "common/status.h"
-#include "gutil/macros.h"
+#include "io/fs/file_system.h"
#include "io/fs/file_writer.h"
-#include "io/fs/path.h"
#include "io/fs/s3_file_system.h"
-#include "util/doris_metrics.h"
-
-using Aws::S3::Model::AbortMultipartUploadRequest;
-using Aws::S3::Model::CompletedPart;
-using Aws::S3::Model::CompletedMultipartUpload;
-using Aws::S3::Model::CompleteMultipartUploadRequest;
-using Aws::S3::Model::CreateMultipartUploadRequest;
-using Aws::S3::Model::DeleteObjectRequest;
-using Aws::S3::Model::UploadPartRequest;
-using Aws::S3::Model::UploadPartOutcome;
+#include "io/fs/s3_file_write_bufferpool.h"
+#include "util/runtime_profile.h"
+#include "util/s3_util.h"
namespace doris {
namespace io {
+using namespace Aws::S3::Model;
+using Aws::S3::S3Client;
-// max size of each part when uploading: 5MB
-static const int MAX_SIZE_EACH_PART = 5 * 1024 * 1024;
-static const char* STREAM_TAG = "S3FileWriter";
-
-S3FileWriter::S3FileWriter(Path path, std::shared_ptr<Aws::S3::S3Client> client,
+S3FileWriter::S3FileWriter(Path&& path, std::string&& key, std::shared_ptr<S3Client> client,
const S3Conf& s3_conf, FileSystemSPtr fs)
- : FileWriter(std::move(path), fs), _client(client), _s3_conf(s3_conf) {
- DorisMetrics::instance()->s3_file_open_writing->increment(1);
- DorisMetrics::instance()->s3_file_writer_total->increment(1);
-}
+ : FileWriter(std::move(path), std::move(fs)),
+ _bucket(s3_conf.bucket),
+ _key(std::move(key)),
+ _client(std::move(client)) {}
S3FileWriter::~S3FileWriter() {
- if (_opened) {
- close();
+ if (!_closed) {
+ abort();
Review Comment:
You change the `close()` to `abort()` there. Does it mean that we must call `close()` explicitly, otherwise the write operation may be aborted?
If yes, better to add comment for S3FileWriter
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] ByteYue commented on pull request #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
Posted by "ByteYue (via GitHub)" <gi...@apache.org>.
ByteYue commented on PR #17585:
URL: https://github.com/apache/doris/pull/17585#issuecomment-1517922995
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #17585:
URL: https://github.com/apache/doris/pull/17585#issuecomment-1517953627
PR approved by anyone and no changes requested.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] ByteYue commented on pull request #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
Posted by "ByteYue (via GitHub)" <gi...@apache.org>.
ByteYue commented on PR #17585:
URL: https://github.com/apache/doris/pull/17585#issuecomment-1497019141
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on a diff in pull request #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #17585:
URL: https://github.com/apache/doris/pull/17585#discussion_r1129593858
##########
be/src/io/fs/s3_file_writer.cpp:
##########
@@ -17,233 +17,241 @@
#include "io/fs/s3_file_writer.h"
-#include <aws/core/Aws.h>
#include <aws/core/utils/HashingUtils.h>
+#include <aws/core/utils/memory/stl/AWSStreamFwd.h>
+#include <aws/core/utils/memory/stl/AWSStringStream.h>
#include <aws/s3/S3Client.h>
#include <aws/s3/model/AbortMultipartUploadRequest.h>
#include <aws/s3/model/CompleteMultipartUploadRequest.h>
+#include <aws/s3/model/CompletedPart.h>
#include <aws/s3/model/CreateMultipartUploadRequest.h>
-#include <aws/s3/model/DeleteObjectRequest.h>
-#include <aws/s3/model/DeleteObjectsRequest.h>
-#include <aws/s3/model/GetObjectRequest.h>
#include <aws/s3/model/UploadPartRequest.h>
-#include <fmt/core.h>
-#include <sys/uio.h>
-#include <cerrno>
+#include <atomic>
+#include <memory>
+#include <utility>
-#include "common/compiler_util.h"
+#include "common/logging.h"
#include "common/status.h"
-#include "gutil/macros.h"
-#include "io/fs/file_writer.h"
-#include "io/fs/path.h"
#include "io/fs/s3_file_system.h"
-#include "util/doris_metrics.h"
-
-using Aws::S3::Model::AbortMultipartUploadRequest;
-using Aws::S3::Model::CompletedPart;
-using Aws::S3::Model::CompletedMultipartUpload;
-using Aws::S3::Model::CompleteMultipartUploadRequest;
-using Aws::S3::Model::CreateMultipartUploadRequest;
-using Aws::S3::Model::DeleteObjectRequest;
-using Aws::S3::Model::UploadPartRequest;
-using Aws::S3::Model::UploadPartOutcome;
+#include "io/fs/s3_file_write_bufferpool.h"
+#include "util/runtime_profile.h"
namespace doris {
namespace io {
-
-// max size of each part when uploading: 5MB
-static const int MAX_SIZE_EACH_PART = 5 * 1024 * 1024;
-static const char* STREAM_TAG = "S3FileWriter";
-
-S3FileWriter::S3FileWriter(Path path, std::shared_ptr<Aws::S3::S3Client> client,
- const S3Conf& s3_conf)
- : FileWriter(std::move(path)), _client(client), _s3_conf(s3_conf) {
- DorisMetrics::instance()->s3_file_open_writing->increment(1);
- DorisMetrics::instance()->s3_file_writer_total->increment(1);
-}
+using namespace Aws::S3::Model;
+using Aws::S3::S3Client;
+
+S3FileWriter::S3FileWriter(Path path, std::string key, std::string bucket,
Review Comment:
warning: constructor for 'doris::io::S3FileWriter' must explicitly initialize the base class 'doris::io::FileWriter' which does not have a default constructor [clang-diagnostic-error]
```cpp
S3FileWriter::S3FileWriter(Path path, std::string key, std::string bucket,
^
```
**be/src/io/fs/file_writer.h:30:** 'doris::io::FileWriter' declared here
```cpp
class FileWriter {
^
```
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on a diff in pull request #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #17585:
URL: https://github.com/apache/doris/pull/17585#discussion_r1129593850
##########
be/src/io/fs/s3_file_write_bufferpool.h:
##########
@@ -0,0 +1,141 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include <condition_variable>
+#include <cstdint>
+#include <fstream>
+#include <functional>
+#include <list>
+#include <memory>
+#include <mutex>
+
+#include "io/fs/s3_common.h"
+#include "common/config.h"
+#include "common/status.h"
+#include "util/slice.h"
+
+namespace doris {
+namespace io {
+
+// TODO(AlexYue): 1. support write into cache 2. unify write buffer and read buffer
+struct S3FileBuffer : public std::enable_shared_from_this<S3FileBuffer> {
Review Comment:
warning: 'S3FileBuffer' defined as a struct here but previously declared as a class; this is valid, but may result in linker errors under the Microsoft C++ ABI [clang-diagnostic-mismatched-tags]
```cpp
struct S3FileBuffer : public std::enable_shared_from_this<S3FileBuffer> {
^
```
**be/src/io/fs/s3_file_writer.h:35:** did you mean struct here?
```cpp
class S3FileBuffer;
^
```
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on a diff in pull request #17585: [feature](io) enable s3 file writer with multi part uploading concurrently
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #17585:
URL: https://github.com/apache/doris/pull/17585#discussion_r1129593877
##########
be/src/io/fs/s3_file_writer.h:
##########
@@ -33,13 +33,17 @@
namespace doris {
namespace io {
+class S3FileBuffer;
class S3FileWriter final : public FileWriter {
public:
- S3FileWriter(Path path, std::shared_ptr<Aws::S3::S3Client> client, const S3Conf& s3_conf);
+ S3FileWriter(Path path, std::string key, std::string bucket, std::shared_ptr<Aws::S3::S3Client>,
+ std::shared_ptr<S3FileSystem> fs, IOContext* state);
~S3FileWriter() override;
- Status close() override;
+ Status open() override;
Review Comment:
warning: only virtual member functions can be marked 'override' [clang-diagnostic-error]
```suggestion
Status open() ;
```
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org