Skip to content

Commit efb7e77

Browse files
Yuhtafacebook-github-bot
authored andcommitted
Allow LocalWriteFile::size to be called after close (facebookincubator#9350)
Summary: Pull Request resolved: facebookincubator#9350 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 fbshipit-source-id: a487a0573a72b2715cc72d5da1663204fa3be062
1 parent 6f8b8cf commit efb7e77

File tree

3 files changed

+12
-6
lines changed

3 files changed

+12
-6
lines changed

velox/common/file/File.cpp

+2-3
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,7 @@ void LocalWriteFile::append(std::string_view data) {
273273
bytesWritten,
274274
data.size(),
275275
folly::errnoStr(errno));
276+
size_ += bytesWritten;
276277
}
277278

278279
void LocalWriteFile::append(std::unique_ptr<folly::IOBuf> data) {
@@ -298,6 +299,7 @@ void LocalWriteFile::append(std::unique_ptr<folly::IOBuf> data) {
298299
"Failure in LocalWriteFile::append, {} vs {}",
299300
totalBytesWritten,
300301
totalBytesToWrite);
302+
size_ += totalBytesWritten;
301303
}
302304

303305
void LocalWriteFile::flush() {
@@ -322,7 +324,4 @@ void LocalWriteFile::close() {
322324
}
323325
}
324326

325-
uint64_t LocalWriteFile::size() const {
326-
return ftell(file_);
327-
}
328327
} // namespace facebook::velox

velox/common/file/File.h

+8-3
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,9 @@ class WriteFile {
155155
// Close the file. Any cleanup (disk flush, etc.) will be done here.
156156
virtual void close() = 0;
157157

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

@@ -283,11 +285,14 @@ class LocalWriteFile final : public WriteFile {
283285
void append(std::unique_ptr<folly::IOBuf> data) final;
284286
void flush() final;
285287
void close() final;
286-
uint64_t size() const final;
288+
289+
uint64_t size() const final {
290+
return size_;
291+
}
287292

288293
private:
289294
FILE* file_;
290-
mutable long size_;
295+
uint64_t size_{0};
291296
bool closed_{false};
292297
};
293298

velox/common/file/tests/FileTest.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,8 @@ TEST(LocalFile, writeAndRead) {
134134
{
135135
LocalWriteFile writeFile(filename);
136136
writeData(&writeFile, useIOBuf);
137+
writeFile.close();
138+
ASSERT_EQ(writeFile.size(), 15 + kOneMB);
137139
}
138140
LocalReadFile readFile(filename);
139141
readData(&readFile);

0 commit comments

Comments
 (0)