You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@heron.apache.org by ni...@apache.org on 2022/05/30 20:48:48 UTC

[incubator-heron] branch master updated: Removed random long in filename which caused leaking in upload storage (#3838)

This is an automated email from the ASF dual-hosted git repository.

nicknezis pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-heron.git


The following commit(s) were added to refs/heads/master by this push:
     new 84bcbbe6c9a Removed random long in filename which caused leaking in upload storage (#3838)
84bcbbe6c9a is described below

commit 84bcbbe6c9adb850ef43c7f83419598d04541900
Author: Nicholas Nezis <ni...@gmail.com>
AuthorDate: Mon May 30 16:48:43 2022 -0400

    Removed random long in filename which caused leaking in upload storage (#3838)
---
 deploy/kubernetes/helm/templates/bookie.yaml              |  3 ---
 deploy/kubernetes/helm/values.yaml.template               |  4 ----
 .../java/org/apache/heron/spi/utils/UploaderUtils.java    |  5 ++---
 .../org/apache/heron/spi/utils/UploaderUtilsTest.java     | 15 +++++++++++----
 4 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/deploy/kubernetes/helm/templates/bookie.yaml b/deploy/kubernetes/helm/templates/bookie.yaml
index 9cc62c81bde..9e6e812feb3 100644
--- a/deploy/kubernetes/helm/templates/bookie.yaml
+++ b/deploy/kubernetes/helm/templates/bookie.yaml
@@ -43,9 +43,6 @@ data:
   BK_indexDirectories: "/bookkeeper/data/ledgers" 
   BK_zkServers: {{ .Release.Name }}-zookeeper:{{ .Values.zookeeper.clientPort }}
   BK_autoRecoveryDaemonEnabled: "true"
-  BK_journalMaxBackups: "{{ .Values.bookkeeper.journal.maxBackups }}"
-  BK_journalMaxSizeMB: "{{ .Values.bookkeeper.journal.maxSizeMB }}"
-  BK_logSizeLimit: "{{ int64 .Values.bookkeeper.logSizeLimit }}"
   {{- if .Values.bookkeeper.useHostNameAsBookieID }}
   BK_useHostNameAsBookieID: "true"
   {{- end }}
diff --git a/deploy/kubernetes/helm/values.yaml.template b/deploy/kubernetes/helm/values.yaml.template
index e02797d40dd..9ffbad7a739 100644
--- a/deploy/kubernetes/helm/values.yaml.template
+++ b/deploy/kubernetes/helm/values.yaml.template
@@ -89,8 +89,6 @@ bookkeeper:
   useStatefulSet: true
   affinityPods: false
   useVolumeClaimTemplate: true
-
-  logSizeLimit: 10000000
   useHostNameAsBookieID: true
   tolerateUnreadyEndpoints: true
 
@@ -108,8 +106,6 @@ bookkeeper:
   useHostPath: false
   journal:
     capacity: 5G
-    maxBackups: 3
-    maxSizeMB: 300
     hostPath: '/opt/bookkeeper/joural-disk'
   ledgers:
     capacity: 15G
diff --git a/heron/spi/src/java/org/apache/heron/spi/utils/UploaderUtils.java b/heron/spi/src/java/org/apache/heron/spi/utils/UploaderUtils.java
index 4365f5a715d..53d16460200 100644
--- a/heron/spi/src/java/org/apache/heron/spi/utils/UploaderUtils.java
+++ b/heron/spi/src/java/org/apache/heron/spi/utils/UploaderUtils.java
@@ -23,7 +23,6 @@ import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
-import java.util.Random;
 
 /**
  * Utility used by Uploader
@@ -65,8 +64,8 @@ public final class UploaderUtils {
       String tag,
       int version,
       String extension) {
-    return String.format("%s-%s-%s-%d-%d%s",
-        topologyName, role, tag, version, new Random().nextLong(), extension);
+    return String.format("%s-%s-%s-%d%s",
+        topologyName, role, tag, version, extension);
   }
 
   public static void copyToOutputStream(String inFile,
diff --git a/heron/spi/tests/java/org/apache/heron/spi/utils/UploaderUtilsTest.java b/heron/spi/tests/java/org/apache/heron/spi/utils/UploaderUtilsTest.java
index f7031b0e870..08958b1388c 100644
--- a/heron/spi/tests/java/org/apache/heron/spi/utils/UploaderUtilsTest.java
+++ b/heron/spi/tests/java/org/apache/heron/spi/utils/UploaderUtilsTest.java
@@ -37,19 +37,26 @@ public class UploaderUtilsTest {
 
   @Test
   public void testGenerateFilename() throws Exception {
-    int expectedUniqueFilename = 10000;
+    int requestsForFilename = 2;
     String topologyName = "topologyName";
     String role = "role";
     String tag = "";
     int version = -1;
     Set<String> filenames = new HashSet<>();
-    for (int i = 0; i < expectedUniqueFilename; i++) {
+    for (int i = 0; i < requestsForFilename; i++) {
       filenames.add(UploaderUtils.generateFilename(
           topologyName, role, tag, version, ""));
     }
 
-    // All filenames should be unique
-    Assert.assertEquals(expectedUniqueFilename, filenames.size());
+    // There should not be multiple entries
+    Assert.assertEquals(1, filenames.size());
+
+    int newVersion = 1;
+    filenames.add(UploaderUtils.generateFilename(
+        topologyName, role, tag, newVersion, ""));
+
+    // Each version should provide a unique filename
+    Assert.assertEquals(2, filenames.size());
   }
 
   @Test