Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CNDB-12425: A few reproduction tests and a preliminary patch, WIP #1529

Draft
wants to merge 14 commits into
base: main
Choose a base branch
from
+1,971 −244
Draft

CNDB-12425: A few reproduction tests and a preliminary patch, WIP #1529

Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
15aa4a3
CNDB-12425: A preliminary patch to facilitate discussions. It does no…
ekaterinadimitrova2 Jan 15, 2025
3bd67f7
Empty-Commit
ekaterinadimitrova2 Feb 9, 2025
3e5d0ba
Make testAllowFilteringDuringIndexBuildsOn3NodeCluster deterministic
ekaterinadimitrova2 Feb 9, 2025
273b51f
A few tiny fixes and spring cleaning
ekaterinadimitrova2 Feb 10, 2025
8d18cb1
Preliminary MV bump, need to check Michael's PR and apply fixes
ekaterinadimitrova2 Feb 12, 2025
5ed5baf
Differentiate between initial build on user index creation and next f…
ekaterinadimitrova2 Mar 2, 2025
a54967a
Fixes needed after rebase
ekaterinadimitrova2 Mar 3, 2025
a086490
Cleaning and adding additional queryability check for Astra
ekaterinadimitrova2 Mar 3, 2025
66331ce
Address review comments
ekaterinadimitrova2 Mar 3, 2025
6202b6d
Small fixes around the messaging version bump
ekaterinadimitrova2 Mar 3, 2025
8c95eb6
Fix Client warnings and use no spam logger. Though we do it on the co…
ekaterinadimitrova2 Mar 5, 2025
9dbd6ae
Moved Gossiper check after peerIndexStatus is populated.
sbtourist Mar 17, 2025
5abaa29
Split AllowFilteringDuringIndexBuildDistributedTest
adelapena Mar 18, 2025
e2ea9dc
Fix and extend AllIndexImplementationsTest
adelapena Mar 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/java/org/apache/cassandra/index/sasi/SASIIndexBuilder.java
View file
Original file line number Diff line number Diff line change
@@ -48,7 +48,7 @@
import org.apache.cassandra.utils.ByteBufferUtil;
import org.apache.cassandra.utils.UUIDGen;

class SASIIndexBuilder extends SecondaryIndexBuilder
public class SASIIndexBuilder extends SecondaryIndexBuilder
{
private final ColumnFamilyStore cfs;
private final UUID compactionId = UUIDGen.getTimeUUID();
111 changes: 83 additions & 28 deletions test/unit/org/apache/cassandra/index/AllIndexImplementationsTest.java
View file
Original file line number Diff line number Diff line change
@@ -19,7 +19,9 @@
import java.util.LinkedList;
import java.util.List;

import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -29,6 +31,7 @@
import org.apache.cassandra.index.internal.CassandraIndex;
import org.apache.cassandra.index.sai.StorageAttachedIndex;
import org.apache.cassandra.index.sasi.SASIIndex;
import org.apache.cassandra.index.sasi.SASIIndexBuilder;
import org.apache.cassandra.inject.Injections;
import org.apache.cassandra.inject.InvokePointBuilder;

@@ -51,42 +54,86 @@ public class AllIndexImplementationsTest extends CQLTester
public String createIndexQuery;

@Parameterized.Parameter(3)
public int useInjection;
public boolean duringInitialBuild;

@Parameterized.Parameters(name = "{0}{3, choice, 0#|1#-with-injection}")
private Injections.Barrier blockIndexBuildInjection;

@Parameterized.Parameters(name = "index={0} during_initial_build={3}")
public static List<Object[]> parameters()
{
List<Object[]> parameters = new LinkedList<>();
// Regular cases without injection
//parameters.add(new Object[]{ "none", null, null, 0 });
//parameters.add(new Object[]{ "legacy", CassandraIndex.class, "CREATE INDEX ON %%s(%s)", 0 });
//parameters.add(new Object[]{ "SASI", SASIIndex.class, "CREATE CUSTOM INDEX ON %%s(%s) USING 'org.apache.cassandra.index.sasi.SASIIndex'", 0 });
//parameters.add(new Object[]{ "SAI", StorageAttachedIndex.class, "CREATE CUSTOM INDEX ON %%s(%s) USING 'StorageAttachedIndex'", 0 });

// Only SAI with injection
parameters.add(new Object[]{ "SAI", StorageAttachedIndex.class, "CREATE CUSTOM INDEX ON %%s(%s) USING 'StorageAttachedIndex'", 1 });

// without any indexes
parameters.add(new Object[]{ "none", null, null, false });

// for each known index implementation, before and after finishing the initial build
for (boolean duringInitialBuild : new boolean[]{ false, true })
{
parameters.add(new Object[]{ "legacy",
CassandraIndex.class,
"CREATE INDEX ON %%s(%s)",
duringInitialBuild });
parameters.add(new Object[]{ "SASI",
SASIIndex.class,
"CREATE CUSTOM INDEX ON %%s(%s) USING 'org.apache.cassandra.index.sasi.SASIIndex'",
duringInitialBuild });
parameters.add(new Object[]{ "SAI",
StorageAttachedIndex.class,
"CREATE CUSTOM INDEX ON %%s(%s) USING 'StorageAttachedIndex'",
duringInitialBuild });
}
return parameters;
}

final Injections.Barrier blockIndexBuild = Injections.newBarrier("block_index_build", 2, false)
.add(InvokePointBuilder.newInvokePoint().onClass(StorageAttachedIndex.class)
.onMethod("startInitialBuild"))
.build();
@Before
public void setupBlockIndexBuildInjection() throws Throwable
{
if (!duringInitialBuild)
return;

@Test
public void testDisjunction() throws Throwable
if (StorageAttachedIndex.class.equals(indexClass))
{
blockIndexBuildInjection = blockInitialBuildInjection(indexClass, "startInitialBuild");
}
else if (CassandraIndex.class.equals(indexClass))
{
blockIndexBuildInjection = blockInitialBuildInjection(indexClass, "buildBlocking");
}
else if (SASIIndex.class.equals(indexClass))
{
blockIndexBuildInjection = blockInitialBuildInjection(SASIIndexBuilder.class, "build");
}
else
{
throw new AssertionError("Unsupported index class: " + indexClass);
}

Injections.inject(blockIndexBuildInjection);
}

@After
public void teardownBlockIndexBuildInjection()
{
if (useInjection == 1 && StorageAttachedIndex.class.equals(indexClass))
if (blockIndexBuildInjection != null)
{
Injections.inject(blockIndexBuild);
blockIndexBuildInjection.countDown();
blockIndexBuildInjection.disable();
}
}

@Test
public void testDisjunction()
{
createTable("CREATE TABLE %s (pk int, a int, b int, PRIMARY KEY(pk))");

boolean indexSupportsDisjuntion = StorageAttachedIndex.class.equals(indexClass);
boolean hasIndex = createIndexQuery != null;
if (hasIndex)
{
createIndexAsync(String.format(createIndexQuery, 'a'));
if (!duringInitialBuild)
waitForTableIndexesQueryable();
}

execute("INSERT INTO %s (pk, a, b) VALUES (?, ?, ?)", 1, 1, 1);
execute("INSERT INTO %s (pk, a, b) VALUES (?, ?, ?)", 2, 2, 2);
@@ -97,7 +144,7 @@ public void testDisjunction() throws Throwable
hasIndex && indexSupportsDisjuntion,
hasIndex ? INDEX_DOES_NOT_SUPPORT_DISJUNCTION : REQUIRES_ALLOW_FILTERING_MESSAGE,
row(1), row(2));
/*assertDisjunction("a = 1 OR b = 2", true, false, row(1), row(2));
assertDisjunction("a = 1 OR b = 2", true, false, row(1), row(2));
assertDisjunction("a = 1 AND (a = 1 OR b = 1)", true, hasIndex, row(1));
assertDisjunction("a = 1 AND (a = 1 OR b = 2)", true, hasIndex, row(1));
assertDisjunction("a = 1 AND (a = 2 OR b = 1)", true, hasIndex, row(1));
@@ -113,11 +160,15 @@ public void testDisjunction() throws Throwable
assertDisjunction("a = 2 OR (a = 1 AND b = 1)", true, indexSupportsDisjuntion, row(1), row(2));
assertDisjunction("a = 2 OR (a = 1 AND b = 2)", true, indexSupportsDisjuntion, row(2));
assertDisjunction("a = 2 OR (a = 2 AND b = 1)", true, indexSupportsDisjuntion, row(2));
assertDisjunction("a = 2 OR (a = 2 AND b = 2)", true, indexSupportsDisjuntion, row(2));*/
assertDisjunction("a = 2 OR (a = 2 AND b = 2)", true, indexSupportsDisjuntion, row(2));

// create a second index in the remaining column, so all columns are indexed
if (hasIndex)
{
createIndexAsync(String.format(createIndexQuery, 'b'));
if (!duringInitialBuild)
waitForTableIndexesQueryable();
}

// test with all columns indexed
assertDisjunction("a = 1 OR a = 2",
@@ -142,12 +193,6 @@ public void testDisjunction() throws Throwable
assertDisjunction("a = 2 OR (a = 1 AND b = 2)", !indexSupportsDisjuntion, indexSupportsDisjuntion, row(2));
assertDisjunction("a = 2 OR (a = 2 AND b = 1)", !indexSupportsDisjuntion, indexSupportsDisjuntion, row(2));
assertDisjunction("a = 2 OR (a = 2 AND b = 2)", !indexSupportsDisjuntion, indexSupportsDisjuntion, row(2));

if (useInjection == 1 && StorageAttachedIndex.class.equals(indexClass))
{
blockIndexBuild.countDown();
blockIndexBuild.disable();
}
}

private void assertDisjunction(String restrictions,
@@ -166,11 +211,13 @@ private void assertDisjunction(String restrictions,
{
// without ALLOW FILTERING
String query = "SELECT pk FROM %s WHERE " + restrictions;

if (requiresFiltering)
assertInvalidThrowMessage(error, InvalidRequestException.class, query);
else if (duringInitialBuild)
assertInvalidThrow(IndexNotAvailableException.class, query);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adelapena , after I rebased on top of CNDB-12620, and I realized that no matter what exception I put here, the tests with all other indexes but SAI always pass...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we don't really need to do the build injections for anything but SAI for two reasons:

  1. all indexes but SAI are always queryable according to Index.isQueryable . Though it seems during the build at least SASI does not return results - https://github.com/riptano/cndb/issues/12931
  2. I think there was a bug in the test and we get actually An index involved in this query does not support disjunctive queries using the OR operator from the first query once I fix it. We were never hitting the if (duringInitialBuild) with non-SAI indexes

else
System.out.println("KATE: I am building it");
//assertRowsIgnoringOrder(execute(query), rows);
assertRowsIgnoringOrder(execute(query), rows);

// with ALLOW FILTERING
query += " ALLOW FILTERING";
@@ -180,4 +227,12 @@ private void assertDisjunction(String restrictions,
Index.QueryPlan plan = parseReadCommand(query).indexQueryPlan();
Assert.assertEquals(shouldUseIndexes, plan != null);
}

private static Injections.Barrier blockInitialBuildInjection(Class<?> indexClass, String methodName)
{
return Injections.newBarrier("block_index_build", 2, false)
.add(InvokePointBuilder.newInvokePoint().onClass(indexClass)
.onMethod(methodName))
.build();
}
}