dixingxing0 commented on pull request #2109: URL: https://github.com/apache/iceberg/pull/2109#issuecomment-763311317
> @dixingxing0 thx a lot for the additional context. that is very helpful. I left a few comments. > > Regarding the scenario of multiple writer jobs and single table, I am afraid that the additional config won't help because we are talking one table here. > > Somehow, we need to allow a provider to provide the suffix for watermark property key. For us, the suffix is the AWS region. I am not sure what is the cleanest way to achieve that. We can define a provider class config and use reflection to instantiate it. I am hesitant with reflection as it is impossible to pass dependency to reflection instantiated class. @stevenzwu thanks for the review and comments! As you described, we cannot config watermark name suffix as table property for multiple writers 😁 . How about we introduce new fields in `FlinkSink.Builder` to config watermark name suffix, this should work for multiple writers: ```java // introduce new fields in org.apache.iceberg.flink.sink.FlinkSink.Builder private boolean storeWatermarkEnabled; // default false private String watermarkNameSuffix; // default "default" // iceberg `table property` or `snapshot summary` written by flink file committer flink.watermark-for-default=the-watermark // use watermarkNameSuffix config as suffix ``` ---------------------------------------------------------------- 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]
