[GitHub] [cassandra] mike-tr-adamson commented on a diff in pull request #2689: CASSANDRA-18737: Do not create sstable files before registering in transaction

["mike-tr-adamson (via GitHub)" <gi...@apache.org>]

mike-tr-adamson commented on code in PR #2689:
URL: https://github.com/apache/cassandra/pull/2689#discussion_r1332836017


##########
src/java/org/apache/cassandra/io/sstable/format/SSTableWriter.java:
##########
@@ -460,26 +515,17 @@ private static Set<Component> indexComponents(Collection<Index.Group> indexGroup
             return components;
         }
 
-        public B addFlushObserversForSecondaryIndexes(Collection<Index.Group> indexGroups, LifecycleNewTracker tracker, TableMetadata metadata)
+        public B addSecondaryIndexGroups(Collection<Index.Group> indexGroups)
         {
             if (indexGroups == null)
                 return (B) this;
 
-            Collection<SSTableFlushObserver> current = this.flushObservers != null ? this.flushObservers : Collections.emptyList();
-            List<SSTableFlushObserver> observers = new ArrayList<>(indexGroups.size() + current.size());
-            observers.addAll(current);
-
-            for (Index.Group group : indexGroups)
-            {
-                SSTableFlushObserver observer = group.getFlushObserver(descriptor, tracker, metadata);
-                if (observer != null)
-                {
-                    observer.begin();
-                    observers.add(observer);
-                }
-            }
+            if (this.indexGroups == null)
+                this.indexGroups = ImmutableList.copyOf(indexGroups);
+            else
+                this.indexGroups = ImmutableList.copyOf(Iterables.concat(this.indexGroups, indexGroups));

Review Comment:
   I'm not sure why we need to do this. I can't see any cases where `addSecondaryIndexGroups` is called twice on a builder. This could be potentially dangerous as we could end up calling the same index group twice (or more).



-- 
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: pr-unsubscribe@cassandra.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org