You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "jrwest (via GitHub)" <gi...@apache.org> on 2023/04/04 16:05:21 UTC

[GitHub] [cassandra] jrwest commented on a diff in pull request #2246: CASSANDRA-18352: Bound user provided timestamps via guardrail

jrwest commented on code in PR #2246:
URL: https://github.com/apache/cassandra/pull/2246#discussion_r1157472841


##########
src/java/org/apache/cassandra/db/guardrails/Guardrails.java:
##########
@@ -421,6 +424,25 @@ public final class Guardrails implements GuardrailsMBean
                      format("The keyspace %s has a replication factor of %s, above the %s threshold of %s.",
                             what, value, isWarning ? "warning" : "failure", threshold));
 
+    public static final MaxThreshold maximumAllowableTimestamp =
+    new MaxThreshold("timestamp_upper_bound",

Review Comment:
   oversight will fix



##########
test/unit/org/apache/cassandra/db/guardrails/GuardrailMaximumTimestampTest.java:
##########
@@ -0,0 +1,84 @@
+/*
+ * 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.db.guardrails;
+
+import java.util.concurrent.TimeUnit;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.cassandra.service.ClientState;
+
+public class GuardrailMaximumTimestampTest extends GuardrailTester
+{
+    public GuardrailMaximumTimestampTest()
+    {
+        // testing multiple guardrails
+        super(Guardrails.maximumAllowableTimestamp);
+    }
+
+    @Before
+    public void setupTest()
+    {
+        createTable("CREATE TABLE IF NOT EXISTS %s (k int primary key, v int)");
+    }
+
+
+    private void setGuardrail(long upperWarnSeconds, long upperFailSeconds)
+    {
+        guardrails().setMaximumTimestampWarnThreshold((int) upperWarnSeconds);
+        guardrails().setMaximumTimestampFailThreshold((int) upperFailSeconds);
+    }
+
+    @Test
+    public void testDisabledAllowsAnyTimestamp() throws Throwable
+    {
+        setGuardrail(0, 0);
+        assertValid("INSERT INTO %s (k, v) VALUES (2, 2) USING TIMESTAMP " + (Long.MAX_VALUE - 1));
+    }
+
+    @Test
+    public void testEnabledFail() throws Throwable
+    {
+        setGuardrail(TimeUnit.DAYS.toSeconds(1),
+                     TimeUnit.DAYS.toSeconds(2));
+        assertFails("INSERT INTO %s (k, v) VALUES (2, 2) USING TIMESTAMP " + (Long.MAX_VALUE - 1), "timestamp_upper_bound violated");
+    }
+
+    @Test
+    public void testEnabledInRange() throws Throwable
+    {
+        setGuardrail(TimeUnit.DAYS.toSeconds(1),
+                     TimeUnit.DAYS.toSeconds(2));
+        assertValid("INSERT INTO %s (k, v) VALUES (1, 1) USING TIMESTAMP " + ClientState.getTimestamp());
+    }
+
+    @Test
+    public void testEnabledWarn() throws Throwable
+    {
+        setGuardrail(TimeUnit.DAYS.toSeconds(1),
+                     TimeUnit.DAYS.toSeconds(2));
+        assertWarns("INSERT INTO %s (k, v) VALUES (1, 1) USING TIMESTAMP " +  (ClientState.getTimestamp() + (TimeUnit.DAYS.toMicros(1) + 4000)),
+                    "timestamp_upper_bound violated");
+    }
+
+
+
+

Review Comment:
   will remove



##########
test/unit/org/apache/cassandra/db/guardrails/GuardrailMinimumTimestampTest.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.db.guardrails;
+
+import java.util.concurrent.TimeUnit;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.cassandra.service.ClientState;
+
+public class GuardrailMinimumTimestampTest extends GuardrailTester
+{
+    public GuardrailMinimumTimestampTest()
+    {
+        // testing multiple guardrails
+        super(Guardrails.minimumAllowableTimestamp);
+    }
+
+    @Before
+    public void setupTest()
+    {
+        createTable("CREATE TABLE IF NOT EXISTS %s (k int primary key, v int)");
+    }
+
+
+    private void setGuardrail(long lowerWarnSeconds, long lowerFailSeconds)
+    {
+        guardrails().setMinimumTimestampWarnThreshold((int) lowerWarnSeconds);
+        guardrails().setMinimumTimestampFailThreshold((int) lowerFailSeconds);
+    }
+
+    @Test
+    public void testDisabled() throws Throwable
+    {
+        setGuardrail(0, 0);
+        assertValid("INSERT INTO %s (k, v) VALUES (1, 1) USING TIMESTAMP 12345");
+    }
+
+    @Test
+    public void testEnabledFailure() throws Throwable
+    {
+        setGuardrail(TimeUnit.DAYS.toSeconds(1),
+                     TimeUnit.DAYS.toSeconds(2));
+        assertFails("INSERT INTO %s (k, v) VALUES (1, 1) USING TIMESTAMP 12345", "timestamp_lower_bound violated");
+    }
+
+    @Test
+    public void testEnabledInRange() throws Throwable
+    {
+        setGuardrail(TimeUnit.DAYS.toSeconds(1),
+                     TimeUnit.DAYS.toSeconds(2));
+        assertValid("INSERT INTO %s (k, v) VALUES (1, 1) USING TIMESTAMP " + ClientState.getTimestamp());
+    }
+
+    @Test
+    public void testEnabledWarn() throws Throwable
+    {
+        setGuardrail(TimeUnit.DAYS.toSeconds(1),
+                     TimeUnit.DAYS.toSeconds(2));
+        assertWarns("INSERT INTO %s (k, v) VALUES (1, 1) USING TIMESTAMP " +  (ClientState.getTimestamp() - (TimeUnit.DAYS.toMicros(1) + 4000)),
+                    "timestamp_lower_bound violated");
+    }
+
+

Review Comment:
   will remove



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