kbendick commented on a change in pull request #1515:
URL: https://github.com/apache/iceberg/pull/1515#discussion_r495526012
##########
File path: flink/src/main/java/org/apache/iceberg/flink/sink/FlinkSink.java
##########
@@ -57,6 +57,9 @@
private static final String ICEBERG_STREAM_WRITER_NAME =
IcebergStreamWriter.class.getSimpleName();
private static final String ICEBERG_FILES_COMMITTER_NAME =
IcebergFilesCommitter.class.getSimpleName();
+ public static final String FLINK_ICEBERG_SINK_FLUSHINTERVAL =
"flink.iceberg.sink.flushinterval";
Review comment:
Can we be sure to document (preferably both in the code as well as in
the docs on the website) that this configuration is only used if checkpointing
is disabled? That's an important point but it seems easy to miss without
looking through the code.
Given the name, as an average developer who uses Flink but possibly does not
know all of the details of its usage and hopefully at least scans the
documentations for various configurations, I imagine I would personally assume
that this defines the flush interval.... full stop. Not the flush interval if
and only if checkpointing is disabled. To me, I would think that the average
Flink developer might liken this to Kafka's `linger.ms` producer configuration
in a sense. Maybe this is just from my background of onboarding developers who
aren't always well versed with "big data" to Flink.
Maybe `flink.iceberg.sink.flushinterval.when.committing.disabled` (or
possibly using dashes for the last part of the name) or something else might be
more clear? Seems a little verbose for me, but as someone who oversees a
variety of developers with differing backgrounds that use Flink, this
configuration property seems likely to cause confusion without further clarity.
I would prefer that we change the name so that reading a comment in the docs
isn't required, but I'm open to both approaches.
Does anybody else have an opinion on this?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
