Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[core] RollingFileWriter#abort shouldn't abort currentWriter twice #4546

Closed
wants to merge 1 commit into from

Conversation

yuzelin
Copy link
Contributor

@yuzelin yuzelin commented Nov 19, 2024

Purpose

For example, currentWriter#write occurs exception E1, it will abort, then the RollingFileWriter#abort will abort currentWriter again. Some OutputStream cannot be closed twice, so it will throw an new exception E2 which covers E1.

Tests

API and Format

Documentation

@wwj6591812
Copy link
Contributor

wwj6591812 commented Nov 19, 2024

I think the second call to RollingFileWriter#abort is in RollingFileWriter#write's catch statement.
But where was the first call to RollingFileWriter#abort?

@yuzelin
Copy link
Contributor Author

yuzelin commented Nov 19, 2024

I think the second call to RollingFileWriter#abort is in RollingFileWriter#write's catch statement. But where was the first call to RollingFileWriter#abort?

I mean RollingFileWriter will call currentWriter#abort when #write has exception, then it will call currentWriter#abort again.

@@ -78,7 +78,12 @@ public void write(T row) throws IOException {
openCurrentWriter();
}

currentWriter.write(row);
Copy link
Contributor

Choose a reason for hiding this comment

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

You modify SingleFileWriter

    @Override
    public void abort() {
        if (out != null) {
            IOUtils.closeQuietly(out);
            out = null;
        }
        fileIO.deleteQuietly(path);
    }

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

@wwj6591812
Copy link
Contributor

I think the second call to RollingFileWriter#abort is in RollingFileWriter#write's catch statement. But where was the first call to RollingFileWriter#abort?

I mean RollingFileWriter will call currentWriter#abort when #write has exception, then it will call currentWriter#abort again.

OK, Thinks.
The first time is in RollingFileWriter#write -> SingleFileWriter#write -> SingleFileWriter#writeImpl -> SingleFileWriter#abort.
The second time is in RollingFileWriter#write -> RollingFileWriter#abort -> SingleFileWriter#abort.

@yuzelin
Copy link
Contributor Author

yuzelin commented Nov 19, 2024

Fixed in #4547

@yuzelin yuzelin closed this Nov 19, 2024
@yuzelin yuzelin deleted the fix_abort branch November 19, 2024 07:54
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