[FLINK-29297] Group Table Store file writers into SingleFileWriter and RollingFileWriter #295
Conversation
JingsongLi
left a comment
JingsongLi
left a comment
There was a problem hiding this comment.
Thanks for the contribution, left comments.
| LOG.warn( | ||
| "Failed to open the bulk writer, closing the output stream and throw the error.", | ||
| e); | ||
| IOUtils.closeQuietly(out); |
There was a problem hiding this comment.
No need to close out here, outside will invoke abort.
| // Abort this writer to clear uncommitted files. | ||
| writer.abort(); | ||
|
|
||
| writer.close(); |
There was a problem hiding this comment.
We don't need to use close and return result.
We can just make RecordWriter.close returns void.
| import java.util.function.Supplier; | ||
|
|
||
| /** A {@link RollingFileWriter} to write {@link KeyValue}s into several rolling data files. */ | ||
| public class KeyValueDataRollingFileWriter extends RollingFileWriter<KeyValue, DataFileMeta> { |
There was a problem hiding this comment.
Do we need to create separate class? I think there is no logical.
| + (currentWriter == null ? null : currentWriter.path()) | ||
| + ". Cleaning up.", | ||
| e); | ||
| abort(); |
There was a problem hiding this comment.
Add document to FileWriter.abort, Implementation needs to be reentrant
| } | ||
|
|
||
| try { | ||
| if (writer == null) { |
There was a problem hiding this comment.
It is a single file writer, the file should be single instead of zero.
We can create writer in the constructor. This can avoid various inconsistencies caused by not producing files.
| + (currentWriter == null ? null : currentWriter.path()) | ||
| + ". Cleaning up.", | ||
| e); | ||
| abort(); |
There was a problem hiding this comment.
Add document to FileWriter.write, clear file by itself now when exception in write.
2 participants
Currently we have two types of files to write:
However, current writers are all based on
MetricFileWriter, which always produces statistics.We'd like to refactor the writers and group them into
SingleFileWriterandRollingFileWriter.StatsCollectingSingleFileWritershould be a subclass ofSingleFileWriterwhich additionally produces statistics, and data file writers should be a subclass ofStatsCollectingSingleFileWriterorRollingFileWriterbased on their level. For extra file writers, extending fromSingleFileWriteris enough.