Skip to content

Conversation

jaydyi
Copy link

@jaydyi jaydyi commented Apr 15, 2022

…art agent

@jaydyi
Copy link
Author

jaydyi commented Apr 15, 2022

if (positionFile.exists() && positionFile.isFile() && positionFile.length() != 0) {
logger.info("Updating position from position file: " + positionFilePath);
loadPositionFile(positionFilePath);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should log a warning in the else condition?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also thought of this at first, but after i read position file operations related logic, i think it's redundant,the reasons are as follows.
When TaildirSource start, operations related to position file include these:

  • ReliableTaildirEventReader initialization, loadPositionFile(positionFilePath) is executed here.
  • PositionWriterRunnable thread launched.
positionWriter = Executors.newSingleThreadScheduledExecutor(
        new ThreadFactoryBuilder().setNameFormat("positionWriter").build());
positionWriter.scheduleWithFixedDelay(new PositionWriterRunnable(),
        writePosInitDelay, writePosInterval, TimeUnit.MILLISECONDS);

positionWriter's job is to read and write position file, and every time it is scheduled to execute, if the file does not exist, it will automatically generate an empty file.
So to sum up, in the startup phase, it doesn't matter whether the file exists or not, so even an warning is not needed, because from the user's point of view, it is normal that the file does not exist when I start it.

@rgoers
Copy link
Member

rgoers commented May 10, 2022

There are 12 open PRs against taildir source. Before applying this I really need to know if this causes any of the other PRs to be moot. Or looking at it from the other perspective, if those PRs are applied what impact will they have on this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants