You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2020/01/22 18:21:08 UTC
[GitHub] [cassandra] ifesdjeen opened a new pull request #428: Cassandra
15505 interceptors
ifesdjeen opened a new pull request #428: Cassandra 15505 interceptors
URL: https://github.com/apache/cassandra/pull/428
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] ifesdjeen commented on a change in pull request #428:
CASSANDRA-15505
Posted by GitBox <gi...@apache.org>.
ifesdjeen commented on a change in pull request #428: CASSANDRA-15505
URL: https://github.com/apache/cassandra/pull/428#discussion_r369974507
##########
File path: test/distributed/org/apache/cassandra/distributed/api/IMatcher.java
##########
@@ -0,0 +1,24 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.distributed.api;
+
+public interface IMatcher
+{
+ public boolean matches(int from, int to, IMessage message);
Review comment:
Sure, we can act on an instance.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] ifesdjeen commented on a change in pull request #428:
CASSANDRA-15505
Posted by GitBox <gi...@apache.org>.
ifesdjeen commented on a change in pull request #428: CASSANDRA-15505
URL: https://github.com/apache/cassandra/pull/428#discussion_r372877968
##########
File path: test/distributed/org/apache/cassandra/distributed/impl/IInvokableInstance.java
##########
@@ -27,6 +27,7 @@
import org.apache.cassandra.distributed.api.IInstance;
import org.apache.cassandra.distributed.api.IIsolatedExecutor;
+import org.apache.cassandra.distributed.api.IMessageFilters;
Review comment:
+1
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] dcapwell commented on a change in pull request #428:
CASSANDRA-15505
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #428: CASSANDRA-15505
URL: https://github.com/apache/cassandra/pull/428#discussion_r369898620
##########
File path: test/distributed/org/apache/cassandra/distributed/impl/MessageFilters.java
##########
@@ -86,65 +69,77 @@ public boolean equals(Object that)
public boolean equals(Filter that)
{
- return Arrays.equals(from, that.from)
- && Arrays.equals(to, that.to)
- && Arrays.equals(verbs, that.verbs);
+ return that.matchers.equals(matchers);
}
- public boolean matches(int from, int to, int verb)
- {
- return (this.from == null || Arrays.binarySearch(this.from, from) >= 0)
- && (this.to == null || Arrays.binarySearch(this.to, to) >= 0)
- && (this.verbs == null || Arrays.binarySearch(this.verbs, verb) >= 0);
- }
-
- public Filter restore()
+ public Filter off()
{
filters.remove(this);
return this;
}
- public Filter drop()
+ public Filter on()
{
filters.add(this);
return this;
}
+
+ public boolean matches(int from, int to, IMessage message)
+ {
+ // Compose matchers using logical AND
+ for (IMatcher matcher : matchers)
+ {
+ if (!matcher.matches(from, to, message))
+ return false;
+ }
+ return true;
+ }
}
public class Builder implements IMessageFilters.Builder
{
- int[] from;
- int[] to;
- int[] verbs;
+ final List<IMatcher> matchers;
+
+ private Builder(int... verbs)
+ {
+ this((from, to, msg) -> Arrays.binarySearch(verbs, msg.verb()) >= 0);
+ }
- private Builder(int[] verbs)
+ private Builder(IMatcher matcher)
{
- this.verbs = verbs;
+ this.matchers = new ArrayList<>(2);
+ this.matchers.add(matcher);
}
public Builder from(int ... nums)
{
- from = nums;
+ this.matchers.add((from, to, msg) -> Arrays.binarySearch(nums, from) >= 0);
return this;
}
public Builder to(int ... nums)
{
- to = nums;
+ this.matchers.add((from, to, msg) -> Arrays.binarySearch(nums, to) >= 0);
return this;
}
- public Filter ready()
+ /**
+ * Every message for which matcher returns `true` will be _dropped_ (assuming all
+ * other matchers in the chain will return `true` as well).
+ */
+ public Builder messagesMatching(IMatcher matcher)
{
- return new Filter(from, to, verbs);
+ this.matchers.add(matcher);
+ return this;
}
public Filter drop()
{
- return ready().drop();
+ return new Filter(matchers).on();
Review comment:
so, you pass in a list of filters, call on which will add the `new Filter` into that list, so we now have a circle graph?
list -> `new Filter` -> list -> `new Filter`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] dcapwell commented on a change in pull request #428:
CASSANDRA-15505
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #428: CASSANDRA-15505
URL: https://github.com/apache/cassandra/pull/428#discussion_r369898305
##########
File path: test/distributed/org/apache/cassandra/distributed/impl/MessageFilters.java
##########
@@ -86,65 +69,77 @@ public boolean equals(Object that)
public boolean equals(Filter that)
{
- return Arrays.equals(from, that.from)
- && Arrays.equals(to, that.to)
- && Arrays.equals(verbs, that.verbs);
+ return that.matchers.equals(matchers);
}
- public boolean matches(int from, int to, int verb)
- {
- return (this.from == null || Arrays.binarySearch(this.from, from) >= 0)
- && (this.to == null || Arrays.binarySearch(this.to, to) >= 0)
- && (this.verbs == null || Arrays.binarySearch(this.verbs, verb) >= 0);
- }
-
- public Filter restore()
+ public Filter off()
{
filters.remove(this);
return this;
}
- public Filter drop()
+ public Filter on()
{
filters.add(this);
return this;
}
+
+ public boolean matches(int from, int to, IMessage message)
+ {
+ // Compose matchers using logical AND
+ for (IMatcher matcher : matchers)
+ {
+ if (!matcher.matches(from, to, message))
+ return false;
+ }
+ return true;
+ }
}
public class Builder implements IMessageFilters.Builder
{
- int[] from;
- int[] to;
- int[] verbs;
+ final List<IMatcher> matchers;
+
+ private Builder(int... verbs)
+ {
+ this((from, to, msg) -> Arrays.binarySearch(verbs, msg.verb()) >= 0);
+ }
- private Builder(int[] verbs)
+ private Builder(IMatcher matcher)
{
- this.verbs = verbs;
+ this.matchers = new ArrayList<>(2);
+ this.matchers.add(matcher);
}
public Builder from(int ... nums)
{
- from = nums;
+ this.matchers.add((from, to, msg) -> Arrays.binarySearch(nums, from) >= 0);
Review comment:
this assumes the input is sorted, but the test case is what calls this; its very possible that the test defined them in non-sorted order
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] dcapwell commented on a change in pull request #428:
CASSANDRA-15505
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #428: CASSANDRA-15505
URL: https://github.com/apache/cassandra/pull/428#discussion_r369892749
##########
File path: test/distributed/org/apache/cassandra/distributed/api/IMatcher.java
##########
@@ -0,0 +1,24 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.distributed.api;
+
+public interface IMatcher
Review comment:
personally feel old name was better, and its the name used for every implementation and call site; why not go back?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] ifesdjeen commented on a change in pull request #428:
CASSANDRA-15505
Posted by GitBox <gi...@apache.org>.
ifesdjeen commented on a change in pull request #428: CASSANDRA-15505
URL: https://github.com/apache/cassandra/pull/428#discussion_r369974556
##########
File path: test/distributed/org/apache/cassandra/distributed/impl/MessageFilters.java
##########
@@ -18,65 +18,48 @@
package org.apache.cassandra.distributed.impl;
+import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Set;
-import java.util.concurrent.CopyOnWriteArraySet;
+import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
-import org.apache.cassandra.distributed.api.IInstance;
+import org.apache.cassandra.distributed.api.IMatcher;
+import org.apache.cassandra.distributed.api.IMessage;
import org.apache.cassandra.distributed.api.IMessageFilters;
import org.apache.cassandra.net.Verb;
-public class MessageFilters implements IMessageFilters
+public class MessageFilters implements IMessageFilters, IMatcher
{
- private final Set<Filter> filters = new CopyOnWriteArraySet<>();
-
- public boolean permit(IInstance from, IInstance to, int verb)
+ private final IMatcher MATCH_ALL = (from, to, msg) -> true;
+ private final List<Filter> filters = new CopyOnWriteArrayList<>();
+
+ /**
+ * {@code true} value returned by this method implies that the message was not matched by any
+ * filters and therefore should be delivered.
+ */
+ public boolean matches(int from, int to, IMessage message)
{
- if (from == null || to == null)
- return false; // cannot deliver
- int fromNum = from.config().num();
- int toNum = to.config().num();
-
for (Filter filter : filters)
- if (filter.matches(fromNum, toNum, verb))
+ {
+ // here, matching logic is inverted: we _drop_ every message that matcher has matched
+ if (filter.matches(from, to, message))
Review comment:
Renamed to `permit`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] dcapwell commented on a change in pull request #428:
CASSANDRA-15505
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #428: CASSANDRA-15505
URL: https://github.com/apache/cassandra/pull/428#discussion_r369894359
##########
File path: test/distributed/org/apache/cassandra/distributed/impl/MessageFilters.java
##########
@@ -18,65 +18,48 @@
package org.apache.cassandra.distributed.impl;
+import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Set;
-import java.util.concurrent.CopyOnWriteArraySet;
+import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
-import org.apache.cassandra.distributed.api.IInstance;
+import org.apache.cassandra.distributed.api.IMatcher;
+import org.apache.cassandra.distributed.api.IMessage;
import org.apache.cassandra.distributed.api.IMessageFilters;
import org.apache.cassandra.net.Verb;
-public class MessageFilters implements IMessageFilters
+public class MessageFilters implements IMessageFilters, IMatcher
{
- private final Set<Filter> filters = new CopyOnWriteArraySet<>();
-
- public boolean permit(IInstance from, IInstance to, int verb)
+ private final IMatcher MATCH_ALL = (from, to, msg) -> true;
+ private final List<Filter> filters = new CopyOnWriteArrayList<>();
+
+ /**
+ * {@code true} value returned by this method implies that the message was not matched by any
Review comment:
you are documenting the implementation where as it should be the interface; that is what should say the expected semantics.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] ifesdjeen commented on a change in pull request #428:
CASSANDRA-15505
Posted by GitBox <gi...@apache.org>.
ifesdjeen commented on a change in pull request #428: CASSANDRA-15505
URL: https://github.com/apache/cassandra/pull/428#discussion_r369974994
##########
File path: test/distributed/org/apache/cassandra/distributed/impl/MessageFilters.java
##########
@@ -86,65 +69,77 @@ public boolean equals(Object that)
public boolean equals(Filter that)
{
- return Arrays.equals(from, that.from)
- && Arrays.equals(to, that.to)
- && Arrays.equals(verbs, that.verbs);
+ return that.matchers.equals(matchers);
}
- public boolean matches(int from, int to, int verb)
- {
- return (this.from == null || Arrays.binarySearch(this.from, from) >= 0)
- && (this.to == null || Arrays.binarySearch(this.to, to) >= 0)
- && (this.verbs == null || Arrays.binarySearch(this.verbs, verb) >= 0);
- }
-
- public Filter restore()
+ public Filter off()
{
filters.remove(this);
Review comment:
So `off` on a filter chain means exactly what it does: removes and disables this specific filter chain. I've added a test to validate the logic, too.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] ifesdjeen commented on a change in pull request #428:
CASSANDRA-15505
Posted by GitBox <gi...@apache.org>.
ifesdjeen commented on a change in pull request #428: CASSANDRA-15505
URL: https://github.com/apache/cassandra/pull/428#discussion_r369974507
##########
File path: test/distributed/org/apache/cassandra/distributed/api/IMatcher.java
##########
@@ -0,0 +1,24 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.distributed.api;
+
+public interface IMatcher
+{
+ public boolean matches(int from, int to, IMessage message);
Review comment:
I have first switched from ints to `Instance` in the patch, but then I've started implementing filters that can execute on instance and if we use instance here, we'll have to make instances serializable as well. I realize having Instance is more powerful but to be honest here I think we might be ok with just an int, at least for now. I just don't know whether passing Instance to other instances is a good idea.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] dcapwell commented on a change in pull request #428:
CASSANDRA-15505
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #428: CASSANDRA-15505
URL: https://github.com/apache/cassandra/pull/428#discussion_r369892961
##########
File path: test/distributed/org/apache/cassandra/distributed/api/IMatcher.java
##########
@@ -0,0 +1,24 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.distributed.api;
+
+public interface IMatcher
+{
+ public boolean matches(int from, int to, IMessage message);
Review comment:
why get rid of the instances in favor of index? Seems more powerful the other way?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] ifesdjeen commented on a change in pull request #428:
CASSANDRA-15505
Posted by GitBox <gi...@apache.org>.
ifesdjeen commented on a change in pull request #428: CASSANDRA-15505
URL: https://github.com/apache/cassandra/pull/428#discussion_r369974507
##########
File path: test/distributed/org/apache/cassandra/distributed/api/IMatcher.java
##########
@@ -0,0 +1,24 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.distributed.api;
+
+public interface IMatcher
+{
+ public boolean matches(int from, int to, IMessage message);
Review comment:
Sure, we can act on an instance. Main reason I didn't do this was unit testing purposes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] ifesdjeen commented on a change in pull request #428:
CASSANDRA-15505
Posted by GitBox <gi...@apache.org>.
ifesdjeen commented on a change in pull request #428: CASSANDRA-15505
URL: https://github.com/apache/cassandra/pull/428#discussion_r372877603
##########
File path: test/distributed/org/apache/cassandra/distributed/impl/MessageFilters.java
##########
@@ -19,28 +19,23 @@
package org.apache.cassandra.distributed.impl;
import java.util.Arrays;
-import java.util.Set;
-import java.util.concurrent.CopyOnWriteArraySet;
+import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
-import org.apache.cassandra.distributed.api.IInstance;
+import org.apache.cassandra.distributed.api.IMessage;
import org.apache.cassandra.distributed.api.IMessageFilters;
-import org.apache.cassandra.net.Verb;
public class MessageFilters implements IMessageFilters
{
- private final Set<Filter> filters = new CopyOnWriteArraySet<>();
+ private final List<Filter> filters = new CopyOnWriteArrayList<>();
- public boolean permit(IInstance from, IInstance to, int verb)
+ public boolean permit(int from, int to, IMessage msg)
{
- if (from == null || to == null)
- return false; // cannot deliver
- int fromNum = from.config().num();
- int toNum = to.config().num();
-
for (Filter filter : filters)
- if (filter.matches(fromNum, toNum, verb))
- return false;
-
+ {
+ if (filter.matches(from, to, msg))
+ return filter.permitIfMatched;
Review comment:
+1
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] yifan-c commented on issue #428: CASSANDRA-15505
Posted by GitBox <gi...@apache.org>.
yifan-c commented on issue #428: CASSANDRA-15505
URL: https://github.com/apache/cassandra/pull/428#issuecomment-577518984
1. The composite `IMatcher` seems complicated. It allows end-users to add multiple filters that match the same condition, but the evaluation stops at the first one that returns `true`, i.e. drop the message. For instance, I can have `filter1: (1, 2, m) -> add_delay; return false`, `filter2:(1, 2, m) -> true`, and `filter3: (1, 2, m) -> add_m_to_list; return true` under `cluster.filters()`. When matching, the evaluation stops at filter2, and ignores filter3. so the code that `add_m_to_list` is not performed.
2. Another thought regarding the goal here. Essentially, it is matching the condition and do interception upon matching. The interception runs **optional** custom code and at the end makes the decision to either return **DROP** or **PASS**. _From this perspective, drop is just a special form of intercept._
a) If allowing multiple filters to match the same condition, there will be multiple decisions made from the corresponding filter. Ideally, there could be a resolver to come up with a final decision. But for simplicity, we can have rule to say **DROP** over **PASS**.
b) If only allowing single filter, the situation is simple.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] dcapwell commented on a change in pull request #428:
CASSANDRA-15505
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #428: CASSANDRA-15505
URL: https://github.com/apache/cassandra/pull/428#discussion_r369895760
##########
File path: test/distributed/org/apache/cassandra/distributed/impl/MessageFilters.java
##########
@@ -86,65 +69,77 @@ public boolean equals(Object that)
public boolean equals(Filter that)
{
- return Arrays.equals(from, that.from)
- && Arrays.equals(to, that.to)
- && Arrays.equals(verbs, that.verbs);
+ return that.matchers.equals(matchers);
}
- public boolean matches(int from, int to, int verb)
- {
- return (this.from == null || Arrays.binarySearch(this.from, from) >= 0)
- && (this.to == null || Arrays.binarySearch(this.to, to) >= 0)
- && (this.verbs == null || Arrays.binarySearch(this.verbs, verb) >= 0);
- }
-
- public Filter restore()
+ public Filter off()
{
filters.remove(this);
Review comment:
was this working before? If I am reading this properly, `this` is an instance of `Filter` which is a composite of other `IMatcher`. If I disable the composite it attempts to remove itself from itself?
ill need to look closer, but this wasn't changed in this so ignoring for now.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] ifesdjeen commented on a change in pull request #428:
CASSANDRA-15505
Posted by GitBox <gi...@apache.org>.
ifesdjeen commented on a change in pull request #428: CASSANDRA-15505
URL: https://github.com/apache/cassandra/pull/428#discussion_r369974556
##########
File path: test/distributed/org/apache/cassandra/distributed/impl/MessageFilters.java
##########
@@ -18,65 +18,48 @@
package org.apache.cassandra.distributed.impl;
+import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Set;
-import java.util.concurrent.CopyOnWriteArraySet;
+import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
-import org.apache.cassandra.distributed.api.IInstance;
+import org.apache.cassandra.distributed.api.IMatcher;
+import org.apache.cassandra.distributed.api.IMessage;
import org.apache.cassandra.distributed.api.IMessageFilters;
import org.apache.cassandra.net.Verb;
-public class MessageFilters implements IMessageFilters
+public class MessageFilters implements IMessageFilters, IMatcher
{
- private final Set<Filter> filters = new CopyOnWriteArraySet<>();
-
- public boolean permit(IInstance from, IInstance to, int verb)
+ private final IMatcher MATCH_ALL = (from, to, msg) -> true;
+ private final List<Filter> filters = new CopyOnWriteArrayList<>();
+
+ /**
+ * {@code true} value returned by this method implies that the message was not matched by any
+ * filters and therefore should be delivered.
+ */
+ public boolean matches(int from, int to, IMessage message)
{
- if (from == null || to == null)
- return false; // cannot deliver
- int fromNum = from.config().num();
- int toNum = to.config().num();
-
for (Filter filter : filters)
- if (filter.matches(fromNum, toNum, verb))
+ {
+ // here, matching logic is inverted: we _drop_ every message that matcher has matched
+ if (filter.matches(from, to, message))
Review comment:
Renamed to `permit`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] ifesdjeen commented on a change in pull request #428:
CASSANDRA-15505
Posted by GitBox <gi...@apache.org>.
ifesdjeen commented on a change in pull request #428: CASSANDRA-15505
URL: https://github.com/apache/cassandra/pull/428#discussion_r372877592
##########
File path: test/distributed/org/apache/cassandra/distributed/impl/MessageFilters.java
##########
@@ -87,70 +86,80 @@ public boolean equals(Object that)
public boolean equals(Filter that)
{
return Arrays.equals(from, that.from)
- && Arrays.equals(to, that.to)
- && Arrays.equals(verbs, that.verbs);
- }
-
- public boolean matches(int from, int to, int verb)
- {
- return (this.from == null || Arrays.binarySearch(this.from, from) >= 0)
- && (this.to == null || Arrays.binarySearch(this.to, to) >= 0)
- && (this.verbs == null || Arrays.binarySearch(this.verbs, verb) >= 0);
+ && Arrays.equals(to, that.to)
+ && Arrays.equals(verbs, that.verbs);
}
- public Filter restore()
+ public Filter off()
{
filters.remove(this);
return this;
}
- public Filter drop()
+ public Filter on()
{
filters.add(this);
return this;
}
+
+ public boolean matches(int from, int to, IMessage msg)
+ {
+ return (this.from == null || Arrays.binarySearch(this.from, from) >= 0)
+ && (this.to == null || Arrays.binarySearch(this.to, to) >= 0)
+ && (this.verbs == null || Arrays.binarySearch(this.verbs, msg.verb()) >= 0)
+ && (this.matcher == null || this.matcher.matches(from, to, msg));
+ }
}
public class Builder implements IMessageFilters.Builder
{
int[] from;
int[] to;
int[] verbs;
+ Matcher onInstance;
Review comment:
+1
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] dcapwell commented on a change in pull request #428:
CASSANDRA-15505
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #428: CASSANDRA-15505
URL: https://github.com/apache/cassandra/pull/428#discussion_r372539598
##########
File path: test/distributed/org/apache/cassandra/distributed/api/IMessageFilters.java
##########
@@ -18,30 +18,37 @@
package org.apache.cassandra.distributed.api;
-import org.apache.cassandra.net.MessagingService;
-import org.apache.cassandra.net.Verb;
+import java.io.Serializable;
public interface IMessageFilters
{
public interface Filter
{
- Filter restore();
- Filter drop();
+ Filter off();
+ Filter on();
}
public interface Builder
{
Builder from(int ... nums);
Builder to(int ... nums);
- Filter ready();
+ Builder messagesMatching(Matcher filter);
Filter drop();
+ Filter pass();
}
- Builder verbs(Verb ... verbs);
+ public interface Matcher extends Serializable
Review comment:
since we don't transfer not needed but doesn't hurt.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] ifesdjeen commented on a change in pull request #428:
CASSANDRA-15505
Posted by GitBox <gi...@apache.org>.
ifesdjeen commented on a change in pull request #428: CASSANDRA-15505
URL: https://github.com/apache/cassandra/pull/428#discussion_r369974507
##########
File path: test/distributed/org/apache/cassandra/distributed/api/IMatcher.java
##########
@@ -0,0 +1,24 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.distributed.api;
+
+public interface IMatcher
+{
+ public boolean matches(int from, int to, IMessage message);
Review comment:
Sure, we can act on an instance. Main reason I didn't do this was unit testing purposes (no need to mock `Instance` this way)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] dcapwell commented on a change in pull request #428:
CASSANDRA-15505
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #428: CASSANDRA-15505
URL: https://github.com/apache/cassandra/pull/428#discussion_r372539941
##########
File path: test/distributed/org/apache/cassandra/distributed/impl/IInvokableInstance.java
##########
@@ -27,6 +27,7 @@
import org.apache.cassandra.distributed.api.IInstance;
import org.apache.cassandra.distributed.api.IIsolatedExecutor;
+import org.apache.cassandra.distributed.api.IMessageFilters;
Review comment:
dead code?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] ifesdjeen commented on a change in pull request #428:
CASSANDRA-15505
Posted by GitBox <gi...@apache.org>.
ifesdjeen commented on a change in pull request #428: CASSANDRA-15505
URL: https://github.com/apache/cassandra/pull/428#discussion_r369974556
##########
File path: test/distributed/org/apache/cassandra/distributed/impl/MessageFilters.java
##########
@@ -18,65 +18,48 @@
package org.apache.cassandra.distributed.impl;
+import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Set;
-import java.util.concurrent.CopyOnWriteArraySet;
+import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
-import org.apache.cassandra.distributed.api.IInstance;
+import org.apache.cassandra.distributed.api.IMatcher;
+import org.apache.cassandra.distributed.api.IMessage;
import org.apache.cassandra.distributed.api.IMessageFilters;
import org.apache.cassandra.net.Verb;
-public class MessageFilters implements IMessageFilters
+public class MessageFilters implements IMessageFilters, IMatcher
{
- private final Set<Filter> filters = new CopyOnWriteArraySet<>();
-
- public boolean permit(IInstance from, IInstance to, int verb)
+ private final IMatcher MATCH_ALL = (from, to, msg) -> true;
+ private final List<Filter> filters = new CopyOnWriteArrayList<>();
+
+ /**
+ * {@code true} value returned by this method implies that the message was not matched by any
+ * filters and therefore should be delivered.
+ */
+ public boolean matches(int from, int to, IMessage message)
{
- if (from == null || to == null)
- return false; // cannot deliver
- int fromNum = from.config().num();
- int toNum = to.config().num();
-
for (Filter filter : filters)
- if (filter.matches(fromNum, toNum, verb))
+ {
+ // here, matching logic is inverted: we _drop_ every message that matcher has matched
+ if (filter.matches(from, to, message))
Review comment:
Moved to `permit`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] dcapwell commented on a change in pull request #428:
CASSANDRA-15505
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #428: CASSANDRA-15505
URL: https://github.com/apache/cassandra/pull/428#discussion_r372542106
##########
File path: test/distributed/org/apache/cassandra/distributed/impl/MessageFilters.java
##########
@@ -19,28 +19,23 @@
package org.apache.cassandra.distributed.impl;
import java.util.Arrays;
-import java.util.Set;
-import java.util.concurrent.CopyOnWriteArraySet;
+import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
-import org.apache.cassandra.distributed.api.IInstance;
+import org.apache.cassandra.distributed.api.IMessage;
import org.apache.cassandra.distributed.api.IMessageFilters;
-import org.apache.cassandra.net.Verb;
public class MessageFilters implements IMessageFilters
{
- private final Set<Filter> filters = new CopyOnWriteArraySet<>();
+ private final List<Filter> filters = new CopyOnWriteArrayList<>();
- public boolean permit(IInstance from, IInstance to, int verb)
+ public boolean permit(int from, int to, IMessage msg)
{
- if (from == null || to == null)
- return false; // cannot deliver
- int fromNum = from.config().num();
- int toNum = to.config().num();
-
for (Filter filter : filters)
- if (filter.matches(fromNum, toNum, verb))
- return false;
-
+ {
+ if (filter.matches(from, to, msg))
+ return filter.permitIfMatched;
Review comment:
this feels weird to me; if I have the following
```
filter(verb).pass();
filter(verb).drop();
```
what happens? It looks like the drop will be skipped in favor of pass. Don't see a test for this since each test calls `filters.reset();` before adding the second filter.
The pass/drop stuff already exists so nothing for this PR, but I feel that `pass()` should be removed (should file jira for this)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] dcapwell commented on a change in pull request #428:
CASSANDRA-15505
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #428: CASSANDRA-15505
URL: https://github.com/apache/cassandra/pull/428#discussion_r369895030
##########
File path: test/distributed/org/apache/cassandra/distributed/impl/MessageFilters.java
##########
@@ -18,65 +18,48 @@
package org.apache.cassandra.distributed.impl;
+import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Set;
-import java.util.concurrent.CopyOnWriteArraySet;
+import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
-import org.apache.cassandra.distributed.api.IInstance;
+import org.apache.cassandra.distributed.api.IMatcher;
+import org.apache.cassandra.distributed.api.IMessage;
import org.apache.cassandra.distributed.api.IMessageFilters;
import org.apache.cassandra.net.Verb;
-public class MessageFilters implements IMessageFilters
+public class MessageFilters implements IMessageFilters, IMatcher
{
- private final Set<Filter> filters = new CopyOnWriteArraySet<>();
-
- public boolean permit(IInstance from, IInstance to, int verb)
+ private final IMatcher MATCH_ALL = (from, to, msg) -> true;
+ private final List<Filter> filters = new CopyOnWriteArrayList<>();
+
+ /**
+ * {@code true} value returned by this method implies that the message was not matched by any
+ * filters and therefore should be delivered.
+ */
+ public boolean matches(int from, int to, IMessage message)
{
- if (from == null || to == null)
- return false; // cannot deliver
- int fromNum = from.config().num();
- int toNum = to.config().num();
-
for (Filter filter : filters)
- if (filter.matches(fromNum, toNum, verb))
+ {
+ // here, matching logic is inverted: we _drop_ every message that matcher has matched
+ if (filter.matches(from, to, message))
Review comment:
this causes me to double take while reading; the method name is `matches` which calls `matches` which acts differently... do feel old name was better
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] ifesdjeen commented on a change in pull request #428:
CASSANDRA-15505
Posted by GitBox <gi...@apache.org>.
ifesdjeen commented on a change in pull request #428: CASSANDRA-15505
URL: https://github.com/apache/cassandra/pull/428#discussion_r372877925
##########
File path: test/distributed/org/apache/cassandra/distributed/impl/MessageFilters.java
##########
@@ -19,28 +19,23 @@
package org.apache.cassandra.distributed.impl;
import java.util.Arrays;
-import java.util.Set;
-import java.util.concurrent.CopyOnWriteArraySet;
+import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
-import org.apache.cassandra.distributed.api.IInstance;
+import org.apache.cassandra.distributed.api.IMessage;
import org.apache.cassandra.distributed.api.IMessageFilters;
-import org.apache.cassandra.net.Verb;
public class MessageFilters implements IMessageFilters
{
- private final Set<Filter> filters = new CopyOnWriteArraySet<>();
+ private final List<Filter> filters = new CopyOnWriteArrayList<>();
- public boolean permit(IInstance from, IInstance to, int verb)
+ public boolean permit(int from, int to, IMessage msg)
{
- if (from == null || to == null)
- return false; // cannot deliver
- int fromNum = from.config().num();
- int toNum = to.config().num();
-
for (Filter filter : filters)
- if (filter.matches(fromNum, toNum, verb))
- return false;
-
+ {
+ if (filter.matches(from, to, msg))
+ return filter.permitIfMatched;
Review comment:
We also can achieve the same thing by simply returning `false` and not matching on custom matcher.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] ifesdjeen commented on a change in pull request #428:
CASSANDRA-15505
Posted by GitBox <gi...@apache.org>.
ifesdjeen commented on a change in pull request #428: CASSANDRA-15505
URL: https://github.com/apache/cassandra/pull/428#discussion_r370036470
##########
File path: test/distributed/org/apache/cassandra/distributed/impl/MessageFilters.java
##########
@@ -86,65 +69,77 @@ public boolean equals(Object that)
public boolean equals(Filter that)
{
- return Arrays.equals(from, that.from)
- && Arrays.equals(to, that.to)
- && Arrays.equals(verbs, that.verbs);
+ return that.matchers.equals(matchers);
}
- public boolean matches(int from, int to, int verb)
- {
- return (this.from == null || Arrays.binarySearch(this.from, from) >= 0)
- && (this.to == null || Arrays.binarySearch(this.to, to) >= 0)
- && (this.verbs == null || Arrays.binarySearch(this.verbs, verb) >= 0);
- }
-
- public Filter restore()
+ public Filter off()
{
filters.remove(this);
return this;
}
- public Filter drop()
+ public Filter on()
{
filters.add(this);
return this;
}
+
+ public boolean matches(int from, int to, IMessage message)
+ {
+ // Compose matchers using logical AND
+ for (IMatcher matcher : matchers)
+ {
+ if (!matcher.matches(from, to, message))
+ return false;
+ }
+ return true;
+ }
}
public class Builder implements IMessageFilters.Builder
{
- int[] from;
- int[] to;
- int[] verbs;
+ final List<IMatcher> matchers;
+
+ private Builder(int... verbs)
+ {
+ this((from, to, msg) -> Arrays.binarySearch(verbs, msg.verb()) >= 0);
+ }
- private Builder(int[] verbs)
+ private Builder(IMatcher matcher)
{
- this.verbs = verbs;
+ this.matchers = new ArrayList<>(2);
+ this.matchers.add(matcher);
}
public Builder from(int ... nums)
{
- from = nums;
+ this.matchers.add((from, to, msg) -> Arrays.binarySearch(nums, from) >= 0);
return this;
}
public Builder to(int ... nums)
{
- to = nums;
+ this.matchers.add((from, to, msg) -> Arrays.binarySearch(nums, to) >= 0);
return this;
}
- public Filter ready()
+ /**
+ * Every message for which matcher returns `true` will be _dropped_ (assuming all
+ * other matchers in the chain will return `true` as well).
+ */
+ public Builder messagesMatching(IMatcher matcher)
{
- return new Filter(from, to, verbs);
+ this.matchers.add(matcher);
+ return this;
}
public Filter drop()
{
- return ready().drop();
+ return new Filter(matchers).on();
Review comment:
Just looking at the patch may make it seem confusing, since it's actually two separate classes. `Builder` has a `List<Matcher>`, which describes its child matchers. When it combines these matchers into a "sum" matcher, it adds this "sum" matcher (all child matchers of a specific filter appended to each other with `&&`) to `filters` field of `MessageFilters`. I still see no descripancy here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] dcapwell commented on a change in pull request #428:
CASSANDRA-15505
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #428: CASSANDRA-15505
URL: https://github.com/apache/cassandra/pull/428#discussion_r370288907
##########
File path: test/distributed/org/apache/cassandra/distributed/api/IMatcher.java
##########
@@ -0,0 +1,24 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.distributed.api;
+
+public interface IMatcher
+{
+ public boolean matches(int from, int to, IMessage message);
Review comment:
serializability makes sense. +1
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] ifesdjeen commented on a change in pull request #428:
CASSANDRA-15505
Posted by GitBox <gi...@apache.org>.
ifesdjeen commented on a change in pull request #428: CASSANDRA-15505
URL: https://github.com/apache/cassandra/pull/428#discussion_r369974457
##########
File path: test/distributed/org/apache/cassandra/distributed/api/IMatcher.java
##########
@@ -0,0 +1,24 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.distributed.api;
+
+public interface IMatcher
Review comment:
Do you mean `permit` here? If yes, just made a change.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] dcapwell commented on a change in pull request #428:
CASSANDRA-15505
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #428: CASSANDRA-15505
URL: https://github.com/apache/cassandra/pull/428#discussion_r372547539
##########
File path: test/distributed/org/apache/cassandra/distributed/impl/MessageFilters.java
##########
@@ -87,70 +86,80 @@ public boolean equals(Object that)
public boolean equals(Filter that)
{
return Arrays.equals(from, that.from)
- && Arrays.equals(to, that.to)
- && Arrays.equals(verbs, that.verbs);
- }
-
- public boolean matches(int from, int to, int verb)
- {
- return (this.from == null || Arrays.binarySearch(this.from, from) >= 0)
- && (this.to == null || Arrays.binarySearch(this.to, to) >= 0)
- && (this.verbs == null || Arrays.binarySearch(this.verbs, verb) >= 0);
+ && Arrays.equals(to, that.to)
+ && Arrays.equals(verbs, that.verbs);
}
- public Filter restore()
+ public Filter off()
{
filters.remove(this);
return this;
}
- public Filter drop()
+ public Filter on()
{
filters.add(this);
return this;
}
+
+ public boolean matches(int from, int to, IMessage msg)
+ {
+ return (this.from == null || Arrays.binarySearch(this.from, from) >= 0)
+ && (this.to == null || Arrays.binarySearch(this.to, to) >= 0)
+ && (this.verbs == null || Arrays.binarySearch(this.verbs, msg.verb()) >= 0)
+ && (this.matcher == null || this.matcher.matches(from, to, msg));
+ }
}
public class Builder implements IMessageFilters.Builder
{
int[] from;
int[] to;
int[] verbs;
+ Matcher onInstance;
Review comment:
nit, since we no longer transfer this is no longer `onInstance`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org