HIVE-29123: Extend ProtobufInputFormat to handle EOFException for par…

[rtrivedi12]

…tially written proto files

What changes were proposed in this pull request?

This PR introduces a more lenient version of ProtobufInputFormat that ignores EOFException when reading partially written proto files.
ProtobufMessageInputFormat skips zero-byte files by creating , which ignores EOFException.

Why are the changes needed?

Abrupt AM termination or OOM failures can result in partially written protobuf files. When reading data from the corresponding external table, these incomplete files may trigger an EOFException, causing the entire query to fail.

To improve resilience, it would be better to gracefully skip unreadable or corrupted proto files and continue reading the remaining valid data, instead of failing the entire query.

Does this PR introduce any user-facing change?

Yes, this would silently skip reading partially written proto files instead of failing with EOFException.

How was this patch tested?

Manually tested with a sample partially written dag data file.

[deniskuzZ]

is that trying to solve same issue as #5983?
cc @Aggarwal-Raghav

[Aggarwal-Raghav]

is that trying to solve same issue as #5983? cc @Aggarwal-Raghav

Based on the description provided in the PR, the issue appears to be different compared to #5983. @rtrivedi12, can you please provide steps for how you created "partially written dag data file." which you used for testing?

[rtrivedi12]

Based on the description provided in the PR, the issue appears to be different compared to #5983. @rtrivedi12, can you please provide steps for how you created "partially written dag data file." which you used for testing?

This EOFException happens much before , in low-level I/O when SequenceFileReader finds end-of-file before completing the read. And I think [#5983] is about InvalidProtocolBufferException that happens when hive successfully reads a record, but the record itself has bad protobuf encoding.

This partially written dag_data file was created by executing a long- running query and killing the yarn application in between which leaves a partially written dag_data file by Tez ProtoHistoryLoggingService.

[abstractdog]

I believe the relationship between this PR and this codepath below should be clarified:

hive/ql/src/java/org/apache/hadoop/hive/ql/io/protobuf/ProtobufMessageInputFormat.java

Lines 67 to 75 in df98c2e

	private RecordReader<K, ProtoMessageWritable<V>> getSafeRecordReader(InputSplit split,
	JobConf job, Reporter reporter) throws IOException {
	try {
	return super.getRecordReader(split, job, reporter);
	} catch (EOFException e) {
	// Ignore EOFException, we create an empty reader for this, instead of failing.
	return null;
	}
	}

If this PR calls the new RecordReader a SafeRecordReader, then maybe we need to change it to distinguish between the two "safe" things:

1. here we can call it IgnoreEOFProtoMessageRecordReader
2. create class javadoc comment on IgnoreEOFProtoMessageRecordReader to explicitly say that this record reader simply returns false from the next method when it reaches EOF during read (which is not the same what the parent does)

[rtrivedi12]

I believe the relationship between this PR and this codepath below should be clarified:

hive/ql/src/java/org/apache/hadoop/hive/ql/io/protobuf/ProtobufMessageInputFormat.java

Lines 67 to 75 in df98c2e

	private RecordReader<K, ProtoMessageWritable<V>> getSafeRecordReader(InputSplit split,
	JobConf job, Reporter reporter) throws IOException {
	try {
	return super.getRecordReader(split, job, reporter);
	} catch (EOFException e) {
	// Ignore EOFException, we create an empty reader for this, instead of failing.
	return null;
	}
	}

If this PR calls the new RecordReader a SafeRecordReader, then maybe we need to change it to distinguish between the two "safe" things:

1. here we can call it IgnoreEOFProtoMessageRecordReader
2. create class javadoc comment on IgnoreEOFProtoMessageRecordReader to explicitly say that this record reader simply returns false from the next method when it reaches EOF during read (which is not the same what the parent does)

Good point, missed it! It may be confusing with Parent SafeRecordReader; I will rename and clarify in the Java doc.

[sonarqubecloud]

Quality Gate passed

Issues
74 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[abstractdog]

+1 LGTM