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

Jump to bottom

SOLR-13696 Simplify routed alias tests to avoid flakiness, improve debugging #2411

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

SOLR-13696 Simplify routed alias tests to avoid flakiness, improve debugging #2411

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 22 additions & 2 deletions solr/core/src/java/org/apache/solr/handler/admin/ConfigSetsHandler.java
View file
Open in desktop
Expand Up @@ -74,10 +74,26 @@
* A {@link org.apache.solr.request.SolrRequestHandler} for ConfigSets API requests.
*/
public class ConfigSetsHandler extends RequestHandlerBase implements PermissionNameProvider {
private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());

private static final Boolean unitTest;
static {
Boolean unitTest1;
try {
Class.forName("org.apache.solr.cloud.SolrCloudTestCase");
unitTest1 = true;
log.warn("{} thinks it is running in a unit test. If you see " +
"this message in a production server please check that you have not added SolrCloudTestCase classes " +
"to your server's classpath. If this class is not present, or is present in a default installation, " +
"please report as a bug.", ConfigSetsHandler.class.getCanonicalName());
} catch (ClassNotFoundException ignored) {
unitTest1 = false;
}
unitTest = unitTest1;
}
final public static Boolean DISABLE_CREATE_AUTH_CHECKS = Boolean.getBoolean("solr.disableConfigSetsCreateAuthChecks"); // this is for back compat only
final public static String DEFAULT_CONFIGSET_NAME = "_default";
final public static String AUTOCREATED_CONFIGSET_SUFFIX = ".AUTOCREATED";
private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
protected final CoreContainer coreContainer;
public static long DEFAULT_ZK_TIMEOUT = 300 * 1000;
/**
Expand Down Expand Up @@ -436,7 +452,7 @@ public Map<String, Object> call(SolrQueryRequest req, SolrQueryResponse rsp, Con

Map<String, Object> props = CollectionsHandler.copy(req.getParams().required(), null, NAME);
props.put(BASE_CONFIGSET, baseConfigSetName);
if (!DISABLE_CREATE_AUTH_CHECKS &&
if (!getDisableCreateAuthChecks() &&
!isTrusted(req, h.coreContainer.getAuthenticationPlugin()) &&
isCurrentlyTrusted(h.coreContainer.getZkController().getZkClient(), ZkConfigManager.CONFIGS_ZKNODE + "/" + baseConfigSetName)) {
throw new SolrException(ErrorCode.UNAUTHORIZED, "Can't create a configset with an unauthenticated request from a trusted " + BASE_CONFIGSET);
Expand Down Expand Up @@ -481,6 +497,10 @@ public static ConfigSetOperation get(ConfigSetAction action) {
}
}

private static Boolean getDisableCreateAuthChecks() {
return unitTest ? Boolean.getBoolean("solr.disableConfigSetsCreateAuthChecks") : DISABLE_CREATE_AUTH_CHECKS;
}

@Override
public Name getPermissionName(AuthorizationContext ctx) {
String a = ctx.getParams().get(ConfigSetParams.ACTION);
Expand Down
16 changes: 12 additions & 4 deletions ...ore/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java
View file
Open in desktop
Expand Up @@ -42,6 +42,7 @@
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -64,10 +65,7 @@ public class CategoryRoutedAliasUpdateProcessorTest extends RoutedAliasUpdatePro
private int lastDocId = 0;
private static CloudSolrClient solrClient;
private int numDocsDeletedOrFailed = 0;
// uncomment to create pause for attaching profiler.
// static {
// JOptionPane.showMessageDialog(null,"Ready?");
// }
private static final String origSysprop=System.getProperty("solr.disableConfigSetsCreateAuthChecks");

@Before
public void doBefore() throws Exception {
Expand All @@ -88,8 +86,18 @@ public void doAfter() throws Exception {
}
}

@BeforeClass
public static void setUpClass() {
System.setProperty("solr.disableConfigSetsCreateAuthChecks", "true");
}

@AfterClass
public static void cleanUpAfterClass() throws Exception {
if (origSysprop != null) {
System.setProperty("solr.disableConfigSetsCreateAuthChecks", origSysprop);
} else {
System.getProperties().remove("solr.disableConfigSetsCreateAuthChecks");
}
solrClient = null;
}

Expand Down
114 changes: 68 additions & 46 deletions .../src/test/org/apache/solr/update/processor/DimensionalRoutedAliasUpdateProcessorTest.java
View file
Open in desktop
Expand Up @@ -45,6 +45,7 @@
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -53,6 +54,7 @@
import static org.apache.solr.client.solrj.request.CollectionAdminRequest.createTimeRoutedAlias;

public class DimensionalRoutedAliasUpdateProcessorTest extends RoutedAliasUpdateProcessorTest {
private static final String origSysprop = System.getProperty("solr.disableConfigSetsCreateAuthChecks");

private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());

Expand Down Expand Up @@ -84,8 +86,18 @@ public void doAfter() throws Exception {
shutdownCluster();
}

@BeforeClass
public static void setUpClass() {
System.setProperty("solr.disableConfigSetsCreateAuthChecks", "true");
}

@AfterClass
public static void finish() throws Exception {
public static void cleanUpAfterClass() throws Exception {
if (origSysprop != null) {
System.setProperty("solr.disableConfigSetsCreateAuthChecks", origSysprop);
} else {
System.getProperties().remove("solr.disableConfigSetsCreateAuthChecks");
}
IOUtils.close(solrClient);
solrClient = null;
}
Expand Down Expand Up @@ -125,7 +137,8 @@ public void testTimeCat() throws Exception {
),
ap(
"tabby"
)
),
1
);

addDocsAndCommit(true, newDoc("calico", "2019-07-02T00:00:00Z"));
Expand All @@ -141,11 +154,12 @@ public void testTimeCat() throws Exception {
ap(
"tabby",
"calico"
)
),
2
);

testFailedDocument("shorthair", "2017-10-23T00:00:00Z", "couldn't be routed" );
testFailedDocument("shorthair", "2020-10-23T00:00:00Z", "too far in the future" );
testFailedDocument("shorthair", "2090-10-23T00:00:00Z", "too far in the future" );
testFailedDocument(null, "2019-07-02T00:00:00Z", "Route value is null");
testFailedDocument("foo__CRA__bar", "2019-07-02T00:00:00Z", "7 character sequence __CRA__");
testFailedDocument("fóóCRAóóbar", "2019-07-02T00:00:00Z", "7 character sequence __CRA__");
Expand All @@ -161,7 +175,8 @@ public void testTimeCat() throws Exception {
ap(
"tabby",
"calico"
)
),
2
);

// 4 docs no new collections
Expand All @@ -183,7 +198,8 @@ public void testTimeCat() throws Exception {
ap(
"tabby",
"calico"
)
),
6
);

// 4 docs 2 new collections, in random order and maybe not using the alias
Expand All @@ -206,7 +222,8 @@ public void testTimeCat() throws Exception {
ap(
"tabby",
"calico"
)
),
10
);

// now test with async pre-create.
Expand Down Expand Up @@ -236,7 +253,8 @@ public void testTimeCat() throws Exception {
"shorthair",
"tabby",
"calico"
)
),
12
);

addDocsAndCommit(false,
Expand Down Expand Up @@ -268,7 +286,8 @@ Here we need to be testing that something that should not be created (extra pree
"shorthair",
"tabby",
"calico"
)
),
14
);

// now test with auto-delete.
Expand Down Expand Up @@ -298,7 +317,8 @@ Here we need to be testing that something that should not be created (extra pree
"shorthair",
"tabby",
"calico"
)
),
16
);

// have to only send to alias here since one of the collections will be deleted.
Expand Down Expand Up @@ -326,25 +346,30 @@ Here we need to be testing that something that should not be created (extra pree
"shorthair",
"tabby",
"calico"
)
), 18
);

}

private void checkDocsInRightCollections(int expected) throws SolrServerException, IOException {
// verify that all the documents ended up in the right collections.
QueryResponse resp = solrClient.query(getAlias(), params(
"q", "*:*",
"rows", "100",
"fl","*,[shard]",
"fl", "*,[shard]",
"sort", "id asc"
));
SolrDocumentList results = resp.getResults();
assertEquals(18, results.getNumFound());
assertEquals(expected, results.getNumFound());
for (SolrDocument result : results) {
String shard = String.valueOf(result.getFieldValue("[shard]"));
String cat = String.valueOf(result.getFieldValue("cat_s"));
Date date = (Date) result.getFieldValue("timestamp_dt");
String day = date.toInstant().toString().split("T")[0];
assertTrue(shard.contains(cat));
assertTrue(shard.contains(day));
assertTrue("Cat=" + cat + " should not be in Shard=" + shard +
" DocId=" + result.getFieldValue("id"), shard.contains(cat));
assertTrue("Day=" + day + " should not be in Shard=" + shard +
" DocId=" + result.getFieldValue("id"), shard.contains(day));
}
}

Expand Down Expand Up @@ -383,7 +408,8 @@ public void testCatTime() throws Exception {
),
ap(
"tabby"
)
),
1
);

addDocsAndCommit(true, newDoc("calico", "2019-07-02T00:00:00Z"));
Expand All @@ -399,11 +425,12 @@ public void testCatTime() throws Exception {
ap(
"tabby",
"calico"
)
),
2
);

testFailedDocument("shorthair", "2017-10-23T00:00:00Z", "couldn't be routed" );
testFailedDocument("shorthair", "2020-10-23T00:00:00Z", "too far in the future" );
testFailedDocument("shorthair", "2090-10-23T00:00:00Z", "too far in the future" );
testFailedDocument(null, "2019-07-02T00:00:00Z", "Route value is null");
testFailedDocument("foo__CRA__bar", "2019-07-02T00:00:00Z", "7 character sequence __CRA__");
testFailedDocument("fóóCRAóóbar", "2019-07-02T00:00:00Z", "7 character sequence __CRA__");
Expand All @@ -419,7 +446,8 @@ public void testCatTime() throws Exception {
ap(
"tabby",
"calico"
)
),
2
);

// 4 docs no new collections
Expand All @@ -441,7 +469,8 @@ public void testCatTime() throws Exception {
ap(
"tabby",
"calico"
)
),
6
);

// 4 docs 2 new collections, in random order and maybe not using the alias
Expand All @@ -466,7 +495,8 @@ public void testCatTime() throws Exception {
ap(
"tabby",
"calico"
)
),
10
);

// now test with async pre-create.
Expand Down Expand Up @@ -496,7 +526,8 @@ public void testCatTime() throws Exception {
"shorthair",
"tabby",
"calico"
)
),
12
);

addDocsAndCommit(false,
Expand Down Expand Up @@ -528,7 +559,8 @@ Here we need to be testing that something that should not be created (extra pree
"shorthair",
"tabby",
"calico"
)
),
14
);

// now test with auto-delete.
Expand Down Expand Up @@ -558,7 +590,8 @@ Here we need to be testing that something that should not be created (extra pree
"shorthair",
"tabby",
"calico"
)
),
16
);

addDocsAndCommit(true,
Expand All @@ -585,27 +618,10 @@ Here we need to be testing that something that should not be created (extra pree
"shorthair",
"tabby",
"calico"
)
),
18
);

// verify that all the documents ended up in the right collections.
QueryResponse resp = solrClient.query(getAlias(), params(
"q", "*:*",
"rows", "100",
"fl","*,[shard]",
"sort", "id asc"
));
SolrDocumentList results = resp.getResults();
assertEquals(18, results.getNumFound());
for (SolrDocument result : results) {
String shard = String.valueOf(result.getFieldValue("[shard]"));
String cat = String.valueOf(result.getFieldValue("cat_s"));
Date date = (Date) result.getFieldValue("timestamp_dt");
String day = date.toInstant().toString().split("T")[0];
assertTrue(shard.contains(cat));
assertTrue(shard.contains(day));
}

}

public String catTimeDraColFor(String category, String timestamp) {
Expand All @@ -621,9 +637,11 @@ public String timeCatDraColFor(String timestamp, String category) {
*
* @param expectedCols the collections we expect to see
* @param categories the categories added thus far
* @param numDocsExpected the expected number of documents across all shards
*/
private void assertCatTimeInvariants(String[] expectedCols, String[] categories) throws Exception {
final int expectNumFound = lastDocId - numDocsDeletedOrFailed; //lastDocId is effectively # generated docs
private void assertCatTimeInvariants(String[] expectedCols, String[] categories, int numDocsExpected) throws Exception {
// verify that all the documents ended up in the right collections.
checkDocsInRightCollections(numDocsExpected);
int totalNumFound = 0;

final List<String> cols = new CollectionAdminRequest.ListAliases().process(solrClient).getAliasesAsLists().get(getSaferTestName());
Expand Down Expand Up @@ -675,7 +693,11 @@ private void assertCatTimeInvariants(String[] expectedCols, String[] categories)

}

assertEquals(expectNumFound, totalNumFound);
assertEquals(numDocsExpected, totalNumFound);

// also sanity check that our concepts of deleted/failed docs and doc id's have remained consistent.
final int properNumFound = lastDocId - numDocsDeletedOrFailed;
assertEquals(properNumFound, totalNumFound);

assertEquals("COLS FOUND:" + cols, expectedCols.length, cols.size());
}
Expand Down