You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "gyfora (via GitHub)" <gi...@apache.org> on 2023/03/07 11:39:28 UTC

[GitHub] [flink-kubernetes-operator] gyfora opened a new pull request, #547: [FLINK-31345] Reduce AutoScalerInfo size by rounding metrics and compression

gyfora opened a new pull request, #547:
URL: https://github.com/apache/flink-kubernetes-operator/pull/547

   ## What is the purpose of the change
   
   Compresses (GZip) the autoscaler info (metrics and scaling history) before storing in the configmap to greately reduce the size and allow keeping longer histories without reaching the 1MB limit.
   
   We also introduce a 3 decimal place rounding for metrics to further reduce size and make the reported metrics nicer.
   
   ## Verifying this change
   
   Change is covered by existing tests.
   
   ## Does this pull request potentially affect one of the following parts:
   
     - Dependencies (does it add or upgrade a dependency): no
     - The public API, i.e., is any changes to the `CustomResourceDescriptors`: no
     - Core observer or reconciler logic that is regularly executed: no
   
   ## Documentation
   
     - Does this pull request introduce a new feature? no
     - If yes, how is the feature documented? not applicable 
   


-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink-kubernetes-operator] mxm commented on a diff in pull request #547: [FLINK-31345] Reduce AutoScalerInfo size by rounding metrics and compression

Posted by "mxm (via GitHub)" <gi...@apache.org>.
mxm commented on code in PR #547:
URL: https://github.com/apache/flink-kubernetes-operator/pull/547#discussion_r1127753735


##########
flink-kubernetes-operator-autoscaler/src/main/java/org/apache/flink/kubernetes/operator/autoscaler/AutoScalerInfo.java:
##########
@@ -119,7 +127,7 @@ public Map<JobVertexID, SortedMap<Instant, ScalingSummary>> getScalingHistory()
         if (scalingHistory != null) {
             return scalingHistory;
         }
-        var yaml = configMap.getData().get(SCALING_HISTORY_KEY);
+        var yaml = decompress(configMap.getData().get(SCALING_HISTORY_KEY));

Review Comment:
   Does that mean all existing ConfigMaps have to be deleted? Could we somehow achieve a graceful migration?



-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink-kubernetes-operator] gyfora commented on a diff in pull request #547: [FLINK-31345] Reduce AutoScalerInfo size by rounding metrics and compression

Posted by "gyfora (via GitHub)" <gi...@apache.org>.
gyfora commented on code in PR #547:
URL: https://github.com/apache/flink-kubernetes-operator/pull/547#discussion_r1127878428


##########
flink-kubernetes-operator-autoscaler/src/main/java/org/apache/flink/kubernetes/operator/autoscaler/AutoScalerInfo.java:
##########
@@ -119,7 +127,7 @@ public Map<JobVertexID, SortedMap<Instant, ScalingSummary>> getScalingHistory()
         if (scalingHistory != null) {
             return scalingHistory;
         }
-        var yaml = configMap.getData().get(SCALING_HISTORY_KEY);
+        var yaml = decompress(configMap.getData().get(SCALING_HISTORY_KEY));

Review Comment:
   Migration should fall back to uncompressed data so it should require no intervention. I just added a test to verify 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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink-kubernetes-operator] gyfora merged pull request #547: [FLINK-31345] Reduce AutoScalerInfo size by rounding metrics and compression

Posted by "gyfora (via GitHub)" <gi...@apache.org>.
gyfora merged PR #547:
URL: https://github.com/apache/flink-kubernetes-operator/pull/547


-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink-kubernetes-operator] mxm commented on a diff in pull request #547: [FLINK-31345] Reduce AutoScalerInfo size by rounding metrics and compression

Posted by "mxm (via GitHub)" <gi...@apache.org>.
mxm commented on code in PR #547:
URL: https://github.com/apache/flink-kubernetes-operator/pull/547#discussion_r1127925272


##########
flink-kubernetes-operator-autoscaler/src/test/java/org/apache/flink/kubernetes/operator/autoscaler/AutoScalerInfoTest.java:
##########
@@ -97,4 +103,45 @@ public void testHistorySizeConfigs() {
                 Set.of(now.plus(Duration.ofSeconds(15))),
                 new AutoScalerInfo(data).getScalingHistory().get(v1).keySet());
     }
+
+    @Test
+    public void testComrpessionMigration() throws JsonProcessingException {

Review Comment:
   ```suggestion
       public void testCompressionMigration() throws JsonProcessingException {
   ```
   
   NIT



-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org