[GitHub] cassandra pull request #278: Avoid running query to self through messaging s...

[aweisberg]

Github user aweisberg commented on a diff in the pull request:

https://github.com/apache/cassandra/pull/278#discussion_r223876124
  
--- Diff: src/java/org/apache/cassandra/net/MessagingService.java ---
@@ -1078,7 +1078,7 @@ public void sendOneWay(MessageOut message, int id, 
InetAddressAndPort to)
 logger.trace("{} sending {} to {}@{}", 
FBUtilities.getBroadcastAddressAndPort(), message.verb, id, to);
 
 if (to.equals(FBUtilities.getBroadcastAddressAndPort()))
-logger.trace("Message-to-self {} going over MessagingService", 
message);
+logger.debug("Message-to-self {} going over MessagingService", 
message);
--- End diff --

This really needs to be NoSpamLogger? How many times a second might we do 
this in some cases? What if new code comes that causes this to occur many times 
a second?

In OSS debug logging is on all the time. That's really not a risk we should 
be taking.


---

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

[GitHub] cassandra pull request #276: Repair job tests

[ifesdjeen]

Github user ifesdjeen closed the pull request at:

https://github.com/apache/cassandra/pull/276


---

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

[GitHub] cassandra pull request #278: Avoid running query to self through messaging s...

[ifesdjeen]

Github user ifesdjeen commented on a diff in the pull request:

https://github.com/apache/cassandra/pull/278#discussion_r223868092
  
--- Diff: 
src/java/org/apache/cassandra/service/reads/repair/AbstractReadRepair.java ---
@@ -102,12 +104,24 @@ void sendReadCommand(Replica to, ReadCallback 
readCallback, boolean speculative)
 else type = to.isFull() ? "full" : "transient";
 Tracing.trace("Enqueuing {} data read to {}", type, to);
 }
-MessageOut message = command.createMessage();
-// if enabled, request additional info about repaired data from 
any full replicas
-if (command.isTrackingRepairedStatus() && to.isFull())
-message = 
message.withParameter(ParameterType.TRACK_REPAIRED_DATA, 
MessagingService.ONE_BYTE);
 
-MessagingService.instance().sendRRWithFailure(message, 
to.endpoint(), readCallback);
+if (to.isSelf())
+{
+try (ReadExecutionController executionController = 
command.executionController();
--- End diff --

Yes, you're absolutely right: it is wrong to block this thread for I/O. We 
in fact have a pattern in order to deal with these things on local execution 
path: 
https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/service/reads/AbstractReadExecutor.java#L157
 
https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/service/reads/ShortReadPartitionsProtection.java#L186
 

Thanks for pointing that out!


---

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

[GitHub] cassandra-dtest pull request #39: Add no-read assert to read-repair test

[ifesdjeen]

Github user ifesdjeen commented on a diff in the pull request:

https://github.com/apache/cassandra-dtest/pull/39#discussion_r223734988
  
--- Diff: read_repair_test.py ---
@@ -495,6 +495,9 @@ def test_normal_read_repair(self):
 assert storage_proxy.speculated_rr_read == 0
 assert storage_proxy.speculated_rr_write == 0
 
+warn = node2.grep_log("Message-to-self TYPE:READ")
--- End diff --

Ultimately - yes. But unfortunately there are still a couple of places we 
still do that. Maybe we should widen the scope of the ticket and check more 
places where this is happening. Or open a separate issue.


---

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