Avoid running query to self through messaging service#278
Avoid running query to self through messaging service#278ifesdjeen wants to merge 4 commits intoapache:trunkfrom ifesdjeen:avoid-querying-self-through-ms
Conversation
|
|
||
| if (to.equals(FBUtilities.getBroadcastAddressAndPort())) | ||
| logger.trace("Message-to-self {} going over MessagingService", message); | ||
| logger.warn("Message-to-self {} going over MessagingService", message); |
There was a problem hiding this comment.
Use NoSpamLogger and maybe for the key incorporate the stack trace so we rate limit every call site separately?
There was a problem hiding this comment.
My last thought is that as a user I can't do much with this information. It's mostly harmless, but I also can't fix it other than by filing a bug report.
I think this is the biggest issue with this change. It will cause people worry when nothing is wrong. I think this should be debug not warn because no action is required on part of a user (as opposed to a developer).
There was a problem hiding this comment.
Right, we can do debug here. I wanted to first throw in this case, but then thought that it's more useful to find all the cases where we still do that and eliminate those, since failing in that case brings more or less nothing.
| MessagingService.instance().sendRRWithFailure(message, to.endpoint(), readCallback); | ||
| if (to.isSelf()) | ||
| { | ||
| try (ReadExecutionController executionController = command.executionController(); |
There was a problem hiding this comment.
Just asking the question, should this occur in this thread or the read stage?
Is there a possibility of this operation succeeding if the local portion times out? If there is maybe it shouldn't be done synchronously in the request stage?
There was a problem hiding this comment.
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!
|
|
||
| if (to.equals(FBUtilities.getBroadcastAddressAndPort())) | ||
| logger.trace("Message-to-self {} going over MessagingService", message); | ||
| logger.debug("Message-to-self {} going over MessagingService", message); |
There was a problem hiding this comment.
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?
Debug logging is on all the time. That's really not a risk we should be taking.
There was a problem hiding this comment.
To be honest, I think relying on the logging might have been a bad idea. I've left this statement with trace and have rewritten the tests instead.
|
Thank you for the review! |
* STAR-158: Add support for multiple SAI on-disk formats (apache#238) * STAR-901 Trie and Lucene style terms point id lookup * STAR-906: Row-awareness for SAI (apache#278) Co-authored-by: Zhao Yang <zhaoyangsingapore@gmail.com> Co-authored-by: Piotr Kołaczkowski <pkolaczk@datastax.com> Co-authored-by: Jason Rutherglen <jasonrutherglen@Jasons-MacBook-Pro.local>
* STAR-158: Add support for multiple SAI on-disk formats (apache#238) * STAR-901 Trie and Lucene style terms point id lookup * STAR-906: Row-awareness for SAI (apache#278) Co-authored-by: Zhao Yang <zhaoyangsingapore@gmail.com> Co-authored-by: Piotr Kołaczkowski <pkolaczk@datastax.com> Co-authored-by: Jason Rutherglen <jasonrutherglen@Jasons-MacBook-Pro.local> (cherry picked from commit 8d9935f)
* STAR-158: Add support for multiple SAI on-disk formats (apache#238) * STAR-901 Trie and Lucene style terms point id lookup * STAR-906: Row-awareness for SAI (apache#278) Co-authored-by: Zhao Yang <zhaoyangsingapore@gmail.com> Co-authored-by: Piotr Kołaczkowski <pkolaczk@datastax.com> Co-authored-by: Jason Rutherglen <jasonrutherglen@Jasons-MacBook-Pro.local> (cherry picked from commit 8d9935f) (cherry picked from commit 854b299)
* STAR-158: Add support for multiple SAI on-disk formats (apache#238) * STAR-901 Trie and Lucene style terms point id lookup * STAR-906: Row-awareness for SAI (apache#278) Co-authored-by: Zhao Yang <zhaoyangsingapore@gmail.com> Co-authored-by: Piotr Kołaczkowski <pkolaczk@datastax.com> Co-authored-by: Jason Rutherglen <jasonrutherglen@Jasons-MacBook-Pro.local> (cherry picked from commit 8d9935f) (cherry picked from commit 854b299) (cherry picked from commit d895ae6)
* STAR-158: Add support for multiple SAI on-disk formats (apache#238) * STAR-901 Trie and Lucene style terms point id lookup * STAR-906: Row-awareness for SAI (apache#278) Co-authored-by: Zhao Yang <zhaoyangsingapore@gmail.com> Co-authored-by: Piotr Kołaczkowski <pkolaczk@datastax.com> Co-authored-by: Jason Rutherglen <jasonrutherglen@Jasons-MacBook-Pro.local> (cherry picked from commit 8d9935f) (cherry picked from commit 854b299) (cherry picked from commit d895ae6) (cherry picked from commit 3395487) (cherry picked from commit c407c2e)
* STAR-158: Add support for multiple SAI on-disk formats (apache#238) * STAR-901 Trie and Lucene style terms point id lookup * STAR-906: Row-awareness for SAI (apache#278) Co-authored-by: Zhao Yang <zhaoyangsingapore@gmail.com> Co-authored-by: Piotr Kołaczkowski <pkolaczk@datastax.com> Co-authored-by: Jason Rutherglen <jasonrutherglen@Jasons-MacBook-Pro.local> (cherry picked from commit 8d9935f) (cherry picked from commit 854b299) (cherry picked from commit d895ae6) (cherry picked from commit 3395487) (cherry picked from commit c407c2e) KeyFetcher refactor Cleanup SAI Fixes for some trivial test failures There are still some failures in those classes: org/apache/cassandra/index/sai/disk/v1/SegmentMergerTest.java org/apache/cassandra/index/sai/functional/FailureTest.java org/apache/cassandra/index/sai/view/IndexViewManagerTest.java org/apache/cassandra/index/sai/virtual/IndexesSystemViewTest.java org/apache/cassandra/index/sai/cql/NativeIndexDDLTest.java STAR-121 Align flush method signature used with ByteBuddy to SegmentBuilder
* STAR-158: Add support for multiple SAI on-disk formats (apache#238) * STAR-901 Trie and Lucene style terms point id lookup * STAR-906: Row-awareness for SAI (apache#278) Co-authored-by: Zhao Yang <zhaoyangsingapore@gmail.com> Co-authored-by: Piotr Kołaczkowski <pkolaczk@datastax.com> Co-authored-by: Jason Rutherglen <jasonrutherglen@Jasons-MacBook-Pro.local> (cherry picked from commit 8d9935f) (cherry picked from commit 854b299) (cherry picked from commit d895ae6) (cherry picked from commit 3395487) (cherry picked from commit c407c2e) KeyFetcher refactor Cleanup SAI Fixes for some trivial test failures There are still some failures in those classes: org/apache/cassandra/index/sai/disk/v1/SegmentMergerTest.java org/apache/cassandra/index/sai/functional/FailureTest.java org/apache/cassandra/index/sai/view/IndexViewManagerTest.java org/apache/cassandra/index/sai/virtual/IndexesSystemViewTest.java org/apache/cassandra/index/sai/cql/NativeIndexDDLTest.java STAR-121 Align flush method signature used with ByteBuddy to SegmentBuilder
2 participants
No description provided.