Do not add a ZK store reader per log (#1238)

slackhq · Feb 20, 2025 · efd67fe · efd67fe
1 parent 790e93f
commit efd67fe
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 41 deletions.
diff --git a/astra/src/main/java/com/slack/astra/logstore/search/fieldRedaction/RedactedFieldReader.java b/astra/src/main/java/com/slack/astra/logstore/search/fieldRedaction/RedactedFieldReader.java
@@ -1,7 +1,8 @@
 package com.slack.astra.logstore.search.fieldRedaction;
 
-import com.slack.astra.metadata.fieldredaction.FieldRedactionMetadataStore;
+import com.slack.astra.metadata.fieldredaction.FieldRedactionMetadata;
 import java.io.IOException;
+import java.util.HashMap;
 import org.apache.lucene.codecs.StoredFieldsReader;
 import org.apache.lucene.index.StoredFieldVisitor;
 
@@ -13,17 +14,17 @@
 class RedactedFieldReader extends StoredFieldsReader {
 
   private final StoredFieldsReader in;
-  private final FieldRedactionMetadataStore fieldRedactionMetadataStore;
+  private final HashMap<String, FieldRedactionMetadata> fieldRedactionsMap;
 
   public RedactedFieldReader(
-      StoredFieldsReader in, FieldRedactionMetadataStore fieldRedactionMetadataStore) {
+      StoredFieldsReader in, HashMap<String, FieldRedactionMetadata> fieldRedactionsMap) {
     this.in = in;
-    this.fieldRedactionMetadataStore = fieldRedactionMetadataStore;
+    this.fieldRedactionsMap = fieldRedactionsMap;
   }
 
   @Override
   public StoredFieldsReader clone() {
-    return new RedactedFieldReader(in, fieldRedactionMetadataStore);
+    return new RedactedFieldReader(in, fieldRedactionsMap);
   }
 
   @Override
@@ -38,7 +39,7 @@ public void close() throws IOException {
 
   @Override
   public void document(int docID, StoredFieldVisitor visitor) throws IOException {
-    visitor = new RedactionStoredFieldVisitor(visitor, fieldRedactionMetadataStore);
+    visitor = new RedactionStoredFieldVisitor(visitor, fieldRedactionsMap);
     in.document(docID, visitor);
   }
 }
diff --git a/astra/src/main/java/com/slack/astra/logstore/search/fieldRedaction/RedactionLeafReader.java b/astra/src/main/java/com/slack/astra/logstore/search/fieldRedaction/RedactionLeafReader.java
@@ -1,7 +1,8 @@
 package com.slack.astra.logstore.search.fieldRedaction;
 
-import com.slack.astra.metadata.fieldredaction.FieldRedactionMetadataStore;
+import com.slack.astra.metadata.fieldredaction.FieldRedactionMetadata;
 import java.io.IOException;
+import java.util.HashMap;
 import org.apache.lucene.codecs.StoredFieldsReader;
 import org.apache.lucene.index.LeafReader;
 import org.apache.lucene.index.SortedDocValues;
@@ -14,12 +15,12 @@
  * RedactionStoredFieldVisitor to read in the individual fields.
  */
 class RedactionLeafReader extends SequentialStoredFieldsLeafReader {
-  private final FieldRedactionMetadataStore fieldRedactionMetadataStore;
+  private final HashMap<String, FieldRedactionMetadata> fieldRedactionsMap;
 
   public RedactionLeafReader(
-      LeafReader in, FieldRedactionMetadataStore fieldRedactionMetadataStore) {
+      LeafReader in, HashMap<String, FieldRedactionMetadata> fieldRedactionsMap) {
     super(in);
-    this.fieldRedactionMetadataStore = fieldRedactionMetadataStore;
+    this.fieldRedactionsMap = fieldRedactionsMap;
   }
 
   @Override
@@ -35,13 +36,13 @@ public StoredFields storedFields() throws IOException {
   // RedactionStoredFieldVisitor can be called here or in the RedactedFieldReader
   @Override
   public void document(int docID, StoredFieldVisitor visitor) throws IOException {
-    visitor = new RedactionStoredFieldVisitor(visitor, fieldRedactionMetadataStore);
+    visitor = new RedactionStoredFieldVisitor(visitor, fieldRedactionsMap);
     in.document(docID, visitor);
   }
 
   @Override
   protected StoredFieldsReader doGetSequentialStoredFieldsReader(StoredFieldsReader reader) {
-    return new RedactedFieldReader(reader, fieldRedactionMetadataStore);
+    return new RedactedFieldReader(reader, fieldRedactionsMap);
   }
 
   @Override

diff --git a/...main/java/com/slack/astra/logstore/search/fieldRedaction/RedactionStoredFieldVisitor.java b/...main/java/com/slack/astra/logstore/search/fieldRedaction/RedactionStoredFieldVisitor.java
@@ -3,9 +3,7 @@
 import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.slack.astra.logstore.LogMessage;
-import com.slack.astra.metadata.core.AstraMetadataStoreChangeListener;
 import com.slack.astra.metadata.fieldredaction.FieldRedactionMetadata;
-import com.slack.astra.metadata.fieldredaction.FieldRedactionMetadataStore;
 import java.io.IOException;
 import java.time.Instant;
 import java.util.HashMap;
@@ -24,32 +22,10 @@ class RedactionStoredFieldVisitor extends StoredFieldVisitor {
   private final String redactedValue = "REDACTED";
 
   public RedactionStoredFieldVisitor(
-      final StoredFieldVisitor delegate, FieldRedactionMetadataStore fieldRedactionMetadataStore) {
+      final StoredFieldVisitor delegate,
+      HashMap<String, FieldRedactionMetadata> fieldRedactionsMap) {
     super();
     this.delegate = delegate;
-
-    Map<String, FieldRedactionMetadata> fieldRedactionsMap = new HashMap<>();
-    fieldRedactionMetadataStore
-        .listSync()
-        .forEach(
-            redaction -> {
-              fieldRedactionsMap.put(redaction.getName(), redaction);
-            });
-
-    // TODO - do we need the listener at all if we listsync at the field level anyway?
-    AstraMetadataStoreChangeListener<FieldRedactionMetadata> listener =
-        new AstraMetadataStoreChangeListener() {
-          @Override
-          public void onMetadataStoreChanged(Object model) {
-            fieldRedactionMetadataStore
-                .listSync()
-                .forEach(
-                    redaction -> {
-                      fieldRedactionsMap.put(redaction.getName(), redaction);
-                    });
-          }
-        };
-    fieldRedactionMetadataStore.addListener(listener);
     this.fieldRedactionsMap = fieldRedactionsMap;
   }
 

diff --git a/...c/main/java/com/slack/astra/logstore/search/fieldRedaction/RedactionSubReaderWrapper.java b/...c/main/java/com/slack/astra/logstore/search/fieldRedaction/RedactionSubReaderWrapper.java
@@ -1,6 +1,8 @@
 package com.slack.astra.logstore.search.fieldRedaction;
 
+import com.slack.astra.metadata.fieldredaction.FieldRedactionMetadata;
 import com.slack.astra.metadata.fieldredaction.FieldRedactionMetadataStore;
+import java.util.HashMap;
 import org.apache.lucene.index.FilterDirectoryReader;
 import org.apache.lucene.index.LeafReader;
 
@@ -9,14 +11,22 @@
  * reader, and creates a RedactionLeafReader.
  */
 class RedactionSubReaderWrapper extends FilterDirectoryReader.SubReaderWrapper {
-  private final FieldRedactionMetadataStore fieldRedactionMetadataStore;
+  private final HashMap<String, FieldRedactionMetadata> fieldRedactionsMap;
 
   public RedactionSubReaderWrapper(FieldRedactionMetadataStore fieldRedactionMetadataStore) {
-    this.fieldRedactionMetadataStore = fieldRedactionMetadataStore;
+    this.fieldRedactionsMap = new HashMap<>();
+    if (fieldRedactionMetadataStore != null) {
+      fieldRedactionMetadataStore
+          .listSync()
+          .forEach(
+              redaction -> {
+                fieldRedactionsMap.put(redaction.getName(), redaction);
+              });
+    }
   }
 
   @Override
   public LeafReader wrap(LeafReader reader) {
-    return new RedactionLeafReader(reader, this.fieldRedactionMetadataStore);
+    return new RedactionLeafReader(reader, this.fieldRedactionsMap);
   }
 }