rohitsinha54 commented on code in PR #33474:
URL: https://github.com/apache/beam/pull/33474#discussion_r1943376738


##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/MetricsToCounterUpdateConverter.java:
##########
@@ -111,6 +118,46 @@ public static CounterUpdate fromStringSet(
         .setStringList(stringList);
   }
 
+  public static CounterUpdate fromBoundedTrie(
+      MetricKey key, boolean isCumulative, BoundedTrieData boundedTrieData) {
+    // BoundedTrie uses SET kind metric aggregation which tracks unique 
strings as a trie.

Review Comment:
   When we create the CounterStructuredName for CounterUpdate we need to send 
an aggregation kind it is defined here: 
   
https://source.corp.google.com/piper///depot/google3/google/dataflow/service/v1b3/work_items.proto;l=151;bpv=0;bpt=1
   My thinking on this is: SET aggregation kind represent collection on of 
unique elements.
   Which hold true here as our BoundedTrie implementation is nested hashmap 
i.e. it has unique elements and when n BoundedTrie from from n CounterUpdate is 
'aggregrated' the result is one, containing unique elements from all of them. 
   
   We can use something else like a TRIE itself but unfortunately this 
definition needs to go in CounterUpdate proto and then have a inception push 
for our apis to be released followed up by google-client api release so we 
would not be able to make the definition on time. 
   



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