Skip to content
Open
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -474,13 +474,28 @@ protected void validateTypeChange(String configSet, SchemaField field, FieldType

void deleteStoredSampleDocs(String configSet) {
try {
ensureSystemCollectionExists();
cloudClient().deleteByQuery(BLOB_STORE_ID, "id:" + configSet + "_sample/*", 10);
} catch (IOException | SolrServerException | SolrException exc) {
Comment on lines 475 to 479
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

deleteStoredSampleDocs now auto-creates .system before attempting a delete. If .system is missing, deletion is effectively a no-op, but this will instead create a new system collection during cleanup flows (e.g., SchemaDesignerAPI.cleanupTemp). Consider skipping ensureSystemCollectionExists() here and treating missing collection/blob as nothing to delete (similar to getStoredSampleDocs).

Copilot uses AI. Check for mistakes.
final String excStr = exc.toString();
log.warn("Failed to delete sample docs from blob store for {} due to: {}", configSet, excStr);
}
}

private void ensureSystemCollectionExists() throws IOException, SolrServerException {
if (!zkStateReader().getClusterState().hasCollection(BLOB_STORE_ID)) {
log.info("Creating {} collection for blob storage", BLOB_STORE_ID);
CollectionAdminRequest.createCollection(BLOB_STORE_ID, null, 1, 1).process(cloudClient());
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

Creating the .system collection with a hard-coded replication factor of 1 may be less resilient than Solr's built-in auto-create path (e.g., HttpSolrCall.autoCreateSystemColl uses min(3, liveNodes) for replication factor). Consider deriving the replication factor from live node count or cluster defaults to avoid creating an under-replicated blob store collection on multi-node clusters.

Suggested change
CollectionAdminRequest.createCollection(BLOB_STORE_ID, null, 1, 1).process(cloudClient());
int liveNodes = zkStateReader().getClusterState().getLiveNodes().size();
int replicationFactor = Math.max(1, Math.min(3, liveNodes));
CollectionAdminRequest.createCollection(BLOB_STORE_ID, null, 1, replicationFactor)
.process(cloudClient());

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mean... okay.. I guess I think of the .system as just supporting the schema designer and does it need "all that", but on the other okay...

try {
zkStateReader().waitForState(BLOB_STORE_ID, 30, TimeUnit.SECONDS, Objects::nonNull);
} catch (InterruptedException | TimeoutException e) {
throw new IOException(
"Failed to see created collection " + BLOB_STORE_ID + " reflected in cluster state",
SolrZkClient.checkInterrupted(e));
}
}
}

@SuppressWarnings("unchecked")
List<SolrInputDocument> getStoredSampleDocs(final String configSet) throws IOException {
var request = new GenericSolrRequest(SolrRequest.METHOD.GET, "/blob/" + configSet + "_sample");
Expand All @@ -496,12 +511,23 @@ List<SolrInputDocument> getStoredSampleDocs(final String configSet) throws IOExc
} else return Collections.emptyList();
} catch (SolrServerException e) {
throw new IOException("Failed to lookup stored docs for " + configSet + " due to: " + e);
} catch (SolrException e) {
// Collection not found or blob not found - treat as no documents stored
if (log.isDebugEnabled()) {
log.debug("No stored sample docs found for {}", configSet, e);
}
return Collections.emptyList();
} finally {
IOUtils.closeQuietly(inputStream);
}
}

void storeSampleDocs(final String configSet, List<SolrInputDocument> docs) throws IOException {
try {
ensureSystemCollectionExists();
} catch (SolrServerException e) {
throw new IOException("Failed to ensure .system collection exists", e);
}
Comment on lines 554 to +559
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

This PR adds runtime creation of .system, but the existing schema designer tests always pre-create the blob store collection in @BeforeClass, so the new behavior isn't exercised. Consider updating/adding a test that starts without .system and asserts that storeSampleDocs (and/or other blob interactions) creates it successfully.

Copilot uses AI. Check for mistakes.
docs.forEach(d -> d.removeField(VERSION_FIELD)); // remove _version_ field before storing ...
postDataToBlobStore(
cloudClient(),
Expand Down
Loading