rionmonster commented on PR #27598:
URL: https://github.com/apache/flink/pull/27598#issuecomment-3899939265

   Thinking about this a bit more (and In the interest of reducing the surface 
area of the changes and the number of touch points), I think we can probably 
accomplish this same behavior without explicitly changing the various 
constructors to include the flag. 
   
   Since we only need to be concerned with registration on _live_ collectors I 
believe that may be able to get around adjusting the constructor directly in 
the `CommittableCollector.of()` call (without touching it elsewhere):
   
   ```
   public static <CommT> CommittableCollector<CommT> 
of(SinkCommitterMetricGroup metricGroup) {
       CommittableCollector<CommT> collector = new 
CommittableCollector<>(metricGroup);
       metricGroup.setCurrentPendingCommittablesGauge(collector::getNumPending);
       return collector;
   }
   ```
   
   and returning the previously adjusted constructor to it's original state:
   ```
   /** For deep-copy. */
   CommittableCollector(
           Map<Long, CheckpointCommittableManagerImpl<CommT>> 
checkpointCommittables,
           SinkCommitterMetricGroup metricGroup) {
       this.checkpointCommittables = new 
TreeMap<>(checkNotNull(checkpointCommittables));
       this.metricGroup = metricGroup;
   }
   ```
   
   Thoughts?


-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to