platoneko commented on code in PR #33796:
URL: https://github.com/apache/doris/pull/33796#discussion_r1570066067
##########
be/src/io/fs/hdfs_file_writer.h:
##########
@@ -48,13 +51,32 @@ class HdfsFileWriter final : public FileWriter {
bool closed() const override { return _closed; }
private:
+ // Flush buffer into file cache, **Notice**: this would clear the
underlying buffer
+ Status _flush_buffer();
+ Status append_hdfs_file(std::string_view content);
+ void _write_into_local_file_cache();
+ Status _append(std::string_view content);
+
Path _path;
HdfsHandler* _hdfs_handler = nullptr;
hdfsFile _hdfs_file = nullptr;
std::string _fs_name;
size_t _bytes_appended = 0;
bool _closed = false;
bool _sync_file_data;
+ FileCacheAllocatorBuilder _cache_builder;
+ struct BatchBuffer {
Review Comment:
```suggestion
class BatchBuffer {
```
##########
be/src/io/fs/hdfs_file_writer.h:
##########
@@ -48,13 +51,32 @@ class HdfsFileWriter final : public FileWriter {
bool closed() const override { return _closed; }
private:
+ // Flush buffer into file cache, **Notice**: this would clear the
underlying buffer
Review Comment:
```suggestion
// Flush buffered data into HDFS client and write local file cache if
enabled, **Notice**: this would clear the underlying buffer
```
##########
be/src/io/fs/hdfs_file_writer.h:
##########
@@ -48,13 +51,32 @@ class HdfsFileWriter final : public FileWriter {
bool closed() const override { return _closed; }
private:
+ // Flush buffer into file cache, **Notice**: this would clear the
underlying buffer
+ Status _flush_buffer();
+ Status append_hdfs_file(std::string_view content);
+ void _write_into_local_file_cache();
+ Status _append(std::string_view content);
+
Path _path;
HdfsHandler* _hdfs_handler = nullptr;
hdfsFile _hdfs_file = nullptr;
std::string _fs_name;
size_t _bytes_appended = 0;
bool _closed = false;
bool _sync_file_data;
+ FileCacheAllocatorBuilder _cache_builder;
Review Comment:
```suggestion
std::unique_ptr<FileCacheAllocatorBuilder> _cache_builder; // nullptr if
disable write file cache
```
##########
be/src/io/fs/hdfs_file_writer.cpp:
##########
@@ -26,23 +27,52 @@
#include "common/logging.h"
#include "common/status.h"
+#include "io/cache/block_file_cache.h"
+#include "io/cache/block_file_cache_factory.h"
+#include "io/cache/file_cache_common.h"
#include "io/fs/err_utils.h"
#include "io/fs/hdfs_file_system.h"
#include "io/hdfs_util.h"
#include "service/backend_options.h"
+#include "util/bvar_helper.h"
namespace doris::io {
+bvar::Adder<uint64_t> hdfs_file_writer_total("hdfs_file_writer_total_num");
+bvar::Adder<uint64_t>
hdfs_bytes_written_total("hdfs_file_writer_bytes_written");
+bvar::Adder<uint64_t> hdfs_file_created_total("hdfs_file_writer_file_created");
+bvar::Adder<uint64_t>
hdfs_file_being_written("hdfs_file_writer_file_being_written");
+
+constexpr size_t MB = 1024 * 1024;
Review Comment:
```suggestion
static constexpr size_t MB = 1024 * 1024;
```
##########
be/src/io/cache/file_cache_common.h:
##########
@@ -50,6 +50,19 @@ struct UInt128Wrapper {
bool operator==(const UInt128Wrapper& other) const { return value_ ==
other.value_; }
};
+class BlockFileCache;
+struct FileBlocksHolder;
+using FileBlocksHolderPtr = std::unique_ptr<FileBlocksHolder>;
+
+struct FileCacheAllocatorBuilder {
Review Comment:
The class naming is a bit strange.
--
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-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
