You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "maedhroz (via GitHub)" <gi...@apache.org> on 2023/03/30 18:09:52 UTC

[GitHub] [cassandra] maedhroz opened a new pull request, #2255: CASSANDRA-18370 Avoid loading the preferred IP for BulkLoader streaming

maedhroz opened a new pull request, #2255:
URL: https://github.com/apache/cassandra/pull/2255

   patch by Caleb Rackliffe; reviewed by ? for CASSANDRA-18370
   
   Co-authored-by: Caleb Rackliffe <ca...@gmail.com>
   Co-authored-by: Jon Meredith <jo...@apache.org>


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


[GitHub] [cassandra] maedhroz closed pull request #2255: CASSANDRA-18370 Avoid loading the preferred IP for BulkLoader streaming

Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz closed pull request #2255: CASSANDRA-18370 Avoid loading the preferred IP for BulkLoader streaming
URL: https://github.com/apache/cassandra/pull/2255


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


[GitHub] [cassandra] maedhroz commented on pull request #2255: CASSANDRA-18370 Avoid loading the preferred IP for BulkLoader streaming

Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on PR #2255:
URL: https://github.com/apache/cassandra/pull/2255#issuecomment-1492271771

   https://github.com/apache/cassandra/commit/853ae8c84049be875921a40c9d5924724cc72792


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


[GitHub] [cassandra] maedhroz commented on a diff in pull request #2255: CASSANDRA-18370 Avoid loading the preferred IP for BulkLoader streaming

Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #2255:
URL: https://github.com/apache/cassandra/pull/2255#discussion_r1153618643


##########
src/java/org/apache/cassandra/streaming/StreamConnectionFactory.java:
##########
@@ -26,4 +26,18 @@
 public interface StreamConnectionFactory
 {
     Channel createConnection(OutboundConnectionSettings template, int messagingVersion) throws IOException;
+
+    /** Provide way to disable getPreferredIP() for tools without access to the system keyspace
+     * <p> 
+     * CASSANDRA-17663 moves calls to SystemKeyspace.getPreferredIP() outside of any threads
+     * that are regularly interrupted.  However the streaming subsystem is also used
+     * by the bulk loader tool, which does not have direct access to the local tables
+     * and uses the client metadata/queries to retrieve it.
+     *
+     * @return true if SystemKeyspace.getPreferredIP() should be used when connecting
+     */
+    default boolean supportsPreferredIp()
+    {
+        return true;

Review Comment:
   Note: Setting this to false fails `RepairErrorsTest`, as it should.



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


[GitHub] [cassandra] maedhroz commented on a diff in pull request #2255: CASSANDRA-18370 Avoid loading the preferred IP for BulkLoader streaming

Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #2255:
URL: https://github.com/apache/cassandra/pull/2255#discussion_r1153619029


##########
src/java/org/apache/cassandra/streaming/async/NettyStreamingMessageSender.java:
##########
@@ -238,7 +238,7 @@ public void sendMessage(StreamMessage message)
                 logger.debug("{} Sending {}", createLogTag(session, null), message);
 
             // Supply a preferred IP up-front to avoid trying to get it in the executor thread, which can be interrupted.
-            OutboundConnectionSettings templateWithConnectTo = template.withConnectTo(template.connectTo());
+            OutboundConnectionSettings templateWithConnectTo = factory.supportsPreferredIp() ? template.withConnectTo(template.connectTo()) : template;

Review Comment:
   Path is tested in `SSTableLoaderEncryptionOptionsTest`



##########
src/java/org/apache/cassandra/streaming/async/NettyStreamingMessageSender.java:
##########
@@ -238,7 +238,7 @@ public void sendMessage(StreamMessage message)
                 logger.debug("{} Sending {}", createLogTag(session, null), message);
 
             // Supply a preferred IP up-front to avoid trying to get it in the executor thread, which can be interrupted.
-            OutboundConnectionSettings templateWithConnectTo = template.withConnectTo(template.connectTo());
+            OutboundConnectionSettings templateWithConnectTo = factory.supportsPreferredIp() ? template.withConnectTo(template.connectTo()) : template;

Review Comment:
   Note: This path is tested in `SSTableLoaderEncryptionOptionsTest`.



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


[GitHub] [cassandra] maedhroz commented on pull request #2255: CASSANDRA-18370 Avoid loading the preferred IP for BulkLoader streaming

Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on PR #2255:
URL: https://github.com/apache/cassandra/pull/2255#issuecomment-1490719768

   https://app.circleci.com/pipelines/github/maedhroz/cassandra?branch=CASSANDRA-18370


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