Skip to content

GH-3011: Deny further writes after InternalParquetRecordWriter is aborted#3450

Open
LuciferYang wants to merge 1 commit intoapache:masterfrom
LuciferYang:GH-3011
Open

GH-3011: Deny further writes after InternalParquetRecordWriter is aborted#3450
LuciferYang wants to merge 1 commit intoapache:masterfrom
LuciferYang:GH-3011

Conversation

@LuciferYang
Copy link
Contributor

@LuciferYang LuciferYang commented Mar 17, 2026

Rationale for this change

After a write error (e.g. OOM during page flush), InternalParquetRecordWriter sets its aborted flag to true and re-throws the exception. However, subsequent calls to write() are silently accepted without checking this flag. Since close() skips flushing when aborted is true, all records written after the error are silently discarded — no exception, no warning. Users only discover the data loss when they attempt to read the file later and find records missing.

To clarify: the aborted check in close() does prevent a malformed file from being written (the flush is correctly skipped). The issue is silent data loss — writes appear to succeed but the data is never persisted, which can be difficult to diagnose after the fact.

What changes are included in this PR?

Added an aborted state check at the beginning of write(). If the writer has been aborted due to a previous error, an IOException is thrown immediately with a clear error message, preventing further writes to a writer in an undefined state.

Are these changes tested?

Yes. Added testWriteAfterAbortShouldThrow in TestParquetWriterError that verifies:

  1. Writing to an aborted writer throws IOException with the expected message
  2. close() on an aborted writer completes without throwing

All existing tests in parquet-hadoop pass without modification.

Are there any user-facing changes?

Yes. Users who previously caught write exceptions and continued writing to the same ParquetWriter will now receive an IOException on subsequent write attempts. This is an intentional change to prevent silent data loss — the correct behavior after a write failure is to discard the writer and create a new one.

Closes #3011

@Jiayi-Wang-db
Copy link
Contributor

Hi @LuciferYang , since we check aborted flag before flush, why would the second write produce a corrupted file?

@LuciferYang
Copy link
Contributor Author

Hi @LuciferYang , since we check aborted flag before flush, why would the second write produce a corrupted file?

Hi @Jiayi-Wang-db, thanks for the question!

You're right that the aborted check in close() prevents a corrupted file from being written — the flush is skipped, so the file won't contain malformed data.

The concern here is more about silent data loss. Consider this scenario:

  1. A write fails (e.g., OOM during page flush), and aborted is set to true.
  2. The user catches the exception but continues calling write() — these calls all succeed without error since nothing currently checks the aborted flag.
  3. When close() is called, it silently skips flushing due to the aborted flag.
  4. All records written after the failure are quietly dropped — no exception, no warning.

The user may not realize any data was lost until they read the file later and find records missing. By throwing an IOException immediately on the next write(), we make the failure explicit so the user can handle it right away rather than discovering the problem much later.

@Jiayi-Wang-db
Copy link
Contributor

Hi @LuciferYang , thanks for the clarification. Yes, it seems like it could silently swallow the exception.
However, I’m not sure if it’s the right semantic for the caller to catch an exception, simply ignore it, and continue calling write.
But since we can’t control how the caller uses it, I agree that proactively throwing the exception would be a better way to handle this case.

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.

Should deny further write if ParquetWriter is aborted

2 participants