Skip to content

Commit ebfd563

Browse files
AbingcbcAbingcbc
authored
fix: clear reader when file is really deleted to solve trunctate when inode reused (alibaba#2424)
* fix: clear reader when file is really deleted Change-Id: I5cc995725cc7946668ad14853a793db64504d194 Co-developed-by: Cursor <noreply@cursor.com> * add ut Change-Id: I07a1710c63ff8693e2a6a37994fe0fa0ae2247b9 Co-developed-by: Cursor <noreply@cursor.com> * fix Change-Id: I5c6a2256a1b346912e8c76caa77679bbf50a2157 Co-developed-by: Cursor <noreply@cursor.com> * fix Change-Id: Iae6a1b94aaf30dfffb7f7ec85ae3d1bb66f4f629 Co-developed-by: Cursor <noreply@cursor.com> * fix Change-Id: Ifcc907e9463f4e83364474ddcd689d12c419e654 Co-developed-by: Cursor <noreply@cursor.com> * fix Change-Id: Ie553a57b5a9445f184e11a2fff926ece35c20b74
1 parent dcb29ea commit ebfd563

4 files changed

Lines changed: 103 additions & 16 deletions

File tree

‎core/file_server/event_handler/EventHandler.cpp‎

Lines changed: 46 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -534,7 +534,22 @@ void ModifyHandler::Handle(const Event& event) {
534534
"file inode", readerArray[0]->GetDevInode().inode)("file size",
535535
readerArray[0]->GetFileSize()));
536536
// release fd as quick as possible
537-
readerArray[0]->CloseFilePtr();
537+
bool isDeleted = false;
538+
readerArray[0]->CloseFilePtr(isDeleted);
539+
if (isDeleted) {
540+
LOG_INFO(sLogger,
541+
("file is really deleted", "will be removed from the log reader queue")(
542+
"real path", readerArray[0]->GetRealLogPath())(
543+
"host path",
544+
readerArray[0]->GetHostLogPath())("dev", readerArray[0]->GetDevInode().dev)(
545+
"inode", readerArray[0]->GetDevInode().inode));
546+
// When read with closed reader, will only read cache
547+
if (readerArray[0]->HasDataInCache()) {
548+
ForceReadLogAndPush(readerArray[0]);
549+
}
550+
mDevInodeReaderMap.erase(readerArray[0]->GetDevInode());
551+
readerArray.pop_front();
552+
}
538553
}
539554
}
540555
}
@@ -787,6 +802,7 @@ void ModifyHandler::Handle(const Event& event) {
787802
"config", mConfigName)("log reader queue name", reader->GetHostLogPath())(
788803
"file device", reader->GetDevInode().dev)("file inode", reader->GetDevInode().inode)(
789804
"file size", reader->GetFileSize())("last file position", reader->GetLastFilePos()));
805+
// will remove reader later
790806
reader->CloseFilePtr();
791807
}
792808
}
@@ -827,7 +843,12 @@ void ModifyHandler::Handle(const Event& event) {
827843
"config", mConfigName)("log reader queue name", reader->GetHostLogPath())(
828844
"file device", reader->GetDevInode().dev)("file inode", reader->GetDevInode().inode)(
829845
"file size", reader->GetFileSize()));
830-
reader->CloseFilePtr();
846+
bool isDeleted = false;
847+
reader->CloseFilePtr(isDeleted);
848+
if (isDeleted) {
849+
readerArrayPtr->pop_front();
850+
mDevInodeReaderMap.erase(reader->GetDevInode());
851+
}
831852
} else if (reader->IsContainerStopped()) {
832853
// update container info one more time, ensure file is hold by same cotnainer
833854
if (reader->UpdateContainerInfo() && !reader->IsContainerStopped()) {
@@ -847,7 +868,12 @@ void ModifyHandler::Handle(const Event& event) {
847868
"file device", reader->GetDevInode().dev)(
848869
"file inode", reader->GetDevInode().inode)("file size", reader->GetFileSize()));
849870
ForceReadLogAndPush(reader);
850-
reader->CloseFilePtr();
871+
bool isDeleted = false;
872+
reader->CloseFilePtr(isDeleted);
873+
if (isDeleted) {
874+
readerArrayPtr->pop_front();
875+
mDevInodeReaderMap.erase(reader->GetDevInode());
876+
}
851877
}
852878
}
853879
break;
@@ -900,21 +926,25 @@ void ModifyHandler::Handle(const Event& event) {
900926
"file device", reader->GetDevInode().dev)("file inode", reader->GetDevInode().inode)(
901927
"file size", reader->GetFileSize())("rotator reader pool size", mRotatorReaderMap.size() + 1));
902928
ForceReadLogAndPush(reader);
903-
reader->CloseFilePtr();
904929
readerArrayPtr->pop_front();
905930
mDevInodeReaderMap.erase(reader->GetDevInode());
906-
mRotatorReaderMap[reader->GetDevInode()] = reader;
907-
// need to push modify event again, but without dev inode
908-
// use head dev + inode
909-
Event* ev = new Event(event.GetSource(),
910-
event.GetEventObject(),
911-
event.GetType(),
912-
event.GetWd(),
913-
event.GetCookie(),
914-
(*readerArrayPtr)[0]->GetDevInode().dev,
915-
(*readerArrayPtr)[0]->GetDevInode().inode);
916-
ev->SetConfigName(mConfigName);
917-
LogInput::GetInstance()->PushEventQueue(ev);
931+
// only move reader to rotator reader map when file is not deleted
932+
bool isDeleted = false;
933+
reader->CloseFilePtr(isDeleted);
934+
if (!isDeleted) {
935+
mRotatorReaderMap[reader->GetDevInode()] = reader;
936+
// need to push modify event again, but without dev inode
937+
// use head dev + inode
938+
Event* ev = new Event(event.GetSource(),
939+
event.GetEventObject(),
940+
event.GetType(),
941+
event.GetWd(),
942+
event.GetCookie(),
943+
(*readerArrayPtr)[0]->GetDevInode().dev,
944+
(*readerArrayPtr)[0]->GetDevInode().inode);
945+
ev->SetConfigName(mConfigName);
946+
LogInput::GetInstance()->PushEventQueue(ev);
947+
}
918948
}
919949
}
920950
// if a file is created, and dev inode cannot found(this means it's a new file), create reader for this file, then

‎core/file_server/reader/LogFileReader.cpp‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,8 @@ size_t LogFileReader::BUFFER_SIZE = 1024 * 512; // 512KB
9898

9999
const int64_t kFirstHashKeySeqID = 1;
100100

101+
const string DELETED_FILE_SUFFIX = "(deleted)";
102+
101103
LogFileReader* LogFileReader::CreateLogFileReader(const string& hostLogPathDir,
102104
const string& hostLogPathFile,
103105
const DevInode& devInode,
@@ -1200,6 +1202,11 @@ bool LogFileReader::CloseTimeoutFilePtr(int32_t curTime) {
12001202
}
12011203

12021204
void LogFileReader::CloseFilePtr() {
1205+
bool isDeleted = false;
1206+
CloseFilePtr(isDeleted);
1207+
}
1208+
1209+
void LogFileReader::CloseFilePtr(bool& isDeleted) {
12031210
if (mLogFileOp.IsOpen()) {
12041211
mCache.shrink_to_fit();
12051212
LOG_DEBUG(sLogger, ("start close LogFileReader", mHostLogPath));
@@ -1255,6 +1262,14 @@ void LogFileReader::CloseFilePtr() {
12551262
// always call OnFileClose
12561263
GloablFileDescriptorManager::GetInstance()->OnFileClose(this);
12571264
}
1265+
if (mRealLogPath.length() > DELETED_FILE_SUFFIX.length()
1266+
&& mRealLogPath.compare(
1267+
mRealLogPath.length() - DELETED_FILE_SUFFIX.length(), DELETED_FILE_SUFFIX.length(), DELETED_FILE_SUFFIX)
1268+
== 0) {
1269+
isDeleted = true;
1270+
} else {
1271+
isDeleted = false;
1272+
}
12581273
}
12591274

12601275
uint64_t LogFileReader::GetLogstoreKey() const {

‎core/file_server/reader/LogFileReader.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,7 @@ class LogFileReader {
332332
void SetSymbolicLinkFlag(bool flag) { mSymbolicLinkFlag = flag; }
333333

334334
void CloseFilePtr();
335+
void CloseFilePtr(bool& isDeleted); // return true if file is deleted (only meaningful on Linux)
335336

336337
// void SetLogstoreKey(uint64_t logstoreKey) { mLogstoreKey = logstoreKey; }
337338

‎core/unittest/event_handler/ModifyHandlerUnittest.cpp‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ class ModifyHandlerUnittest : public ::testing::Test {
5959
void TestHandleModifyEventWhenContainerRestartCase5();
6060
void TestHandleModifyEventWhenContainerRestartCase6();
6161
void TestHandleModifyEvnetWhenContainerStopTwice();
62+
void TestClearReaderWhenFileDeleted();
6263

6364
protected:
6465
static void SetUpTestCase() {
@@ -235,6 +236,9 @@ UNIT_TEST_CASE(ModifyHandlerUnittest, TestHandleModifyEventWhenContainerRestartC
235236
UNIT_TEST_CASE(ModifyHandlerUnittest, TestHandleModifyEventWhenContainerRestartCase5);
236237
UNIT_TEST_CASE(ModifyHandlerUnittest, TestHandleModifyEventWhenContainerRestartCase6);
237238
UNIT_TEST_CASE(ModifyHandlerUnittest, TestHandleModifyEvnetWhenContainerStopTwice);
239+
#ifndef _MSC_VER
240+
UNIT_TEST_CASE(ModifyHandlerUnittest, TestClearReaderWhenFileDeleted);
241+
#endif
238242

239243
void ModifyHandlerUnittest::TestHandleContainerStoppedEventWhenReadToEnd() {
240244
LOG_INFO(sLogger, ("TestHandleContainerStoppedEventWhenReadToEnd() begin", time(NULL)));
@@ -799,6 +803,43 @@ void ModifyHandlerUnittest::TestHandleModifyEvnetWhenContainerStopTwice() {
799803
APSARA_TEST_EQUAL_FATAL(mReaderPtr->mContainerID, "2");
800804
}
801805

806+
void ModifyHandlerUnittest::TestClearReaderWhenFileDeleted() {
807+
LOG_INFO(sLogger, ("TestClearReaderWhenFileDeleted() begin", time(NULL)));
808+
809+
// Read log to end to ensure IsReadToEnd() returns true
810+
Event event1(gRootDir, "", EVENT_MODIFY, 0);
811+
LogBuffer logbuf;
812+
APSARA_TEST_TRUE_FATAL(!mReaderPtr->ReadLog(logbuf, &event1)); // false means no more data
813+
APSARA_TEST_TRUE_FATAL(mReaderPtr->mLogFileOp.IsOpen());
814+
815+
// Verify reader is read to end
816+
APSARA_TEST_TRUE_FATAL(mReaderPtr->IsReadToEnd());
817+
818+
// Actually delete the file while file descriptor is still open
819+
// This simulates the real scenario where file is deleted but fd is held
820+
std::string logPath = gRootDir + PATH_SEPARATOR + gLogName;
821+
bfs::remove(logPath);
822+
823+
// Verify reader exists before handling event
824+
APSARA_TEST_EQUAL_FATAL(mHandlerPtr->mDevInodeReaderMap.size(), 1);
825+
APSARA_TEST_EQUAL_FATAL(mHandlerPtr->mNameReaderMap[gLogName].size(), 1);
826+
827+
// Send delete event to trigger close and cleanup
828+
Event event2(gRootDir, gLogName, EVENT_DELETE, 0);
829+
mHandlerPtr->Handle(event2);
830+
831+
// Verify file descriptor is closed
832+
APSARA_TEST_TRUE_FATAL(!mReaderPtr->mLogFileOp.IsOpen());
833+
834+
// Verify reader has been removed from mDevInodeReaderMap
835+
APSARA_TEST_EQUAL_FATAL(mHandlerPtr->mDevInodeReaderMap.size(), 0);
836+
837+
// Verify reader has been removed from readerArray
838+
APSARA_TEST_EQUAL_FATAL(mHandlerPtr->mNameReaderMap[gLogName].size(), 0);
839+
840+
LOG_INFO(sLogger, ("TestClearReaderWhenFileDeleted() end", time(NULL)));
841+
}
842+
802843
} // end of namespace logtail
803844

804845
int main(int argc, char** argv) {

0 commit comments

Comments
 (0)