Hi Jing Ge, Thank you for your valuable comments!
1. I agree with your suggestion regarding following the JavaBean convention. It would be beneficial to incorporate this convention into our Code Style Guide [1]. By doing so, we can ensure consistency and make it easier for developers to adhere to the standard. 2. Yes, you are correct that the results remain the same. From my understanding, this can be considered an implementation detail. 3. By the current design, when objectReuseCompliant = false, it actually signifies an unknown state, and the decision to use object reuse depends on the global configuration, pipeline.object-reuse. And objectReuseCompliant = false is the default value of a StreamOperator. On the other hand, when objectReuseCompliant = true, it indicates that object reuse can be employed. And I don't think we require a value that specifically enforces deep copying since we will enable object reuse for all operators when pipeline.object-reuse = true to maintain the current behavior. Once again, thank you for your input, and let me know if there's anything else I can assist you with. Best regards, Xuannan [1] https://flink.apache.org/how-to-contribute/code-style-and-quality-preamble/ On Wed, Jul 5, 2023 at 1:37 AM Jing Ge <j...@ververica.com.invalid> wrote: > > Hi Xuannan, Hi Dong > > Thanks for the Proposal! After reading the FLIP, I'd like to ask some > questions: > > 1. Naming convention for boolean variables. It is recommended to follow > JavaBean [1], i.e. objectReuseCompliant as the variable name with > isObjectReuseCompliant() and setObjectReuseCompliant() as the methods' name. > > > 2. > > - > > *If pipeline.object-reuse is set to true, records emitted by this > operator will be re-used.* > - > > *Otherwise, if getIsObjectReuseCompliant() returns true, records emitted > by this operator will be re-used.* > - > > *Otherwise, records emitted by this operator will be deep-copied before > being given to the next operator in the chain.* > > > If I understand you correctly, the hard coding objectReusedCompliant > should have higher priority over the configuration, the checking logic > should be: > > - > > *If getIsObjectReuseCompliant() returns true, records emitted by this > operator will be re-used.* > - > > *Otherwise, if pipeline.object-reuse is set to true, records emitted by > this operator will be re-used.* > - > > *Otherwise, records emitted by this operator will be deep-copied before > being given to the next operator in the chain.* > > > The results are the same but the checking logics are different, but there > are some additional thoughts, which lead us to the next question. > > 3. Current design lets specific operators enable object reuse and ignore > the global config. There could be another thought, on the contrary: if an > operator has hard coded the objectReuseCompliant as false, i.e. disable > object reuse on purpose, records should not be reused even if the global > config pipeline.object-reused is set to true, which turns out that the > objectReuseCompliant could be a triple value logic: ture: force object > reusing; false: force deep-copying; unknown: depends on > pipeline.object-reuse config. > > > Best regards, > Jing > > > [1] https://en.wikipedia.org/wiki/JavaBeans > > On Mon, Jul 3, 2023 at 4:25 AM Xuannan Su <suxuanna...@gmail.com> wrote: > > > Hi all, > > > > Dong(cc'ed) and I are opening this thread to discuss our proposal to > > add operator attribute to allow operator to specify support for > > object-reuse [1]. > > > > Currently, the default configuration for pipeline.object-reuse is set > > to false to avoid data corruption, which can result in suboptimal > > performance. We propose adding APIs that operators can utilize to > > inform the Flink runtime whether it is safe to reuse the emitted > > records. This enhancement would enable Flink to maximize its > > performance using the default configuration. > > > > Please refer to the FLIP document for more details about the proposed > > design and implementation. We welcome any feedback and opinions on > > this proposal. > > > > Best regards, > > > > Dong and Xuannan > > > > [1] > > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=255073749 > >
