Skip to content

Commit

Permalink
Allow LocalWriteFile::size to be called after close (facebookincubato…
Browse files Browse the repository at this point in the history
…r#9350)

Summary:

Alpha writer is accessing `size` after the file close.  The only implementation that does not allow this is `LocalWriteFile`.  Fix this in `LocalWriteFile` so this is less likely to causing problem on call sites.

Reviewed By: xiaoxmeng

Differential Revision: D55662422
  • Loading branch information
Yuhta authored and facebook-github-bot committed Apr 3, 2024
1 parent f0f5986 commit f865563
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 6 deletions.
5 changes: 2 additions & 3 deletions velox/common/file/File.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ void LocalWriteFile::append(std::string_view data) {
bytesWritten,
data.size(),
folly::errnoStr(errno));
size_ += bytesWritten;
}

void LocalWriteFile::append(std::unique_ptr<folly::IOBuf> data) {
Expand All @@ -298,6 +299,7 @@ void LocalWriteFile::append(std::unique_ptr<folly::IOBuf> data) {
"Failure in LocalWriteFile::append, {} vs {}",
totalBytesWritten,
totalBytesToWrite);
size_ += totalBytesWritten;
}

void LocalWriteFile::flush() {
Expand All @@ -322,7 +324,4 @@ void LocalWriteFile::close() {
}
}

uint64_t LocalWriteFile::size() const {
return ftell(file_);
}
} // namespace facebook::velox
11 changes: 8 additions & 3 deletions velox/common/file/File.h
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,9 @@ class WriteFile {
// Close the file. Any cleanup (disk flush, etc.) will be done here.
virtual void close() = 0;

// Current file size, i.e. the sum of all previous Appends.
/// Current file size, i.e. the sum of all previous Appends. No flush should
/// be needed to get the exact size written, and this should be able to be
/// called after the file close.
virtual uint64_t size() const = 0;
};

Expand Down Expand Up @@ -283,11 +285,14 @@ class LocalWriteFile final : public WriteFile {
void append(std::unique_ptr<folly::IOBuf> data) final;
void flush() final;
void close() final;
uint64_t size() const final;

uint64_t size() const final {
return size_;
}

private:
FILE* file_;
mutable long size_;
uint64_t size_{0};
bool closed_{false};
};

Expand Down
2 changes: 2 additions & 0 deletions velox/common/file/tests/FileTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ TEST(LocalFile, writeAndRead) {
{
LocalWriteFile writeFile(filename);
writeData(&writeFile, useIOBuf);
writeFile.close();
ASSERT_EQ(writeFile.size(), 15 + kOneMB);
}
LocalReadFile readFile(filename);
readData(&readFile);
Expand Down

0 comments on commit f865563

Please sign in to comment.