Skip to content

Commit 30ce01e

Browse files
kotman12kotman12
committed
refactor
1 parent 9fba72e commit 30ce01e

File tree

2 files changed

+99
-58
lines changed

2 files changed

+99
-58
lines changed

‎solr/core/src/java/org/apache/solr/handler/admin/LukeRequestHandler.java‎

Lines changed: 82 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ public class LukeRequestHandler extends RequestHandlerBase implements SolrCoreAw
136136
private static final String KEY_TYPE = "type";
137137
private static final String KEY_SCHEMA_FLAGS = "schema";
138138
private static final String KEY_DOCS = "docs";
139+
private static final String KEY_DOCS_AS_LONG = "docsAsLong";
139140
private static final String KEY_DISTINCT = "distinct";
140141
private static final String KEY_TOP_TERMS = "topTerms";
141142
private static final String KEY_DYNAMIC_BASE = "dynamicBase";
@@ -244,7 +245,23 @@ && handleDistributed(req, rsp)) {
244245
rsp.setHttpCaching(false);
245246
}
246247

247-
private record FieldOrigin(String shardAddr, LukeResponse.FieldInfo fieldInfo) {}
248+
/** Tracks the first-seen valid properties of a field across shards. */
249+
private static class ExpectedFieldConfig {
250+
final String shardAddr;
251+
final LukeResponse.FieldInfo fieldInfo;
252+
Object indexFlags;
253+
String indexFlagsShardAddr;
254+
255+
ExpectedFieldConfig(String shardAddr, LukeResponse.FieldInfo fieldInfo) {
256+
this.shardAddr = shardAddr;
257+
this.fieldInfo = fieldInfo;
258+
Object flags = fieldInfo.getExtras().get(KEY_INDEX_FLAGS);
259+
if (flags != null) {
260+
this.indexFlags = flags;
261+
this.indexFlagsShardAddr = shardAddr;
262+
}
263+
}
264+
}
248265

249266
/**
250267
* @return true if the request was handled in distributed mode, false if prepDistributed
@@ -301,16 +318,14 @@ private static String shardAddress(ShardResponse srsp) {
301318
return srsp.getShardAddress() != null ? srsp.getShardAddress() : srsp.getShard();
302319
}
303320

304-
@SuppressWarnings("unchecked")
305321
private void mergeDistributedResponses(SolrQueryResponse rsp, List<ShardResponse> responses) {
306322
long totalNumDocs = 0;
307323
int totalMaxDoc = 0;
308324
long totalDeletedDocs = 0;
309325
int totalSegmentCount = 0;
310326

311-
Map<String, SimpleOrderedMap<Object>> fieldLookup = new HashMap<>();
312-
SimpleOrderedMap<Object> fieldsResult = new SimpleOrderedMap<>();
313-
Map<String, FieldOrigin> fieldOrigins = new HashMap<>();
327+
Map<String, SimpleOrderedMap<Object>> mergedFields = new HashMap<>();
328+
Map<String, ExpectedFieldConfig> expectedFieldConfigs = new HashMap<>();
314329
SimpleOrderedMap<Object> shardsInfo = new SimpleOrderedMap<>();
315330

316331
if (!responses.isEmpty()) {
@@ -356,15 +371,29 @@ private void mergeDistributedResponses(SolrQueryResponse rsp, List<ShardResponse
356371
String fieldName = entry.getKey();
357372
LukeResponse.FieldInfo fi = entry.getValue();
358373

359-
SimpleOrderedMap<Object> merged = fieldLookup.get(fieldName);
360-
if (merged == null) {
361-
merged = new SimpleOrderedMap<>();
362-
fieldLookup.put(fieldName, merged);
363-
fieldsResult.add(fieldName, merged);
364-
}
374+
SimpleOrderedMap<Object> merged =
375+
mergedFields.computeIfAbsent(fieldName, k -> new SimpleOrderedMap<>());
376+
377+
mergeShardField(shardAddr, fi, merged, expectedFieldConfigs);
378+
379+
// Detailed stats — kept per-shard, not merged
380+
NamedList<Integer> topTerms = fi.getTopTerms();
381+
Object histogram = fi.getExtras().get(KEY_HISTOGRAM);
365382

366-
mergeShardField(
367-
shardAddr, fi, fieldName, merged, fieldOrigins, perShardFields, perShardEntry);
383+
if (topTerms != null || fi.getDistinct() > 0 || histogram != null) {
384+
perShardEntry.putIfAbsent(RSP_FIELDS, perShardFields);
385+
SimpleOrderedMap<Object> detailedFieldInfo = new SimpleOrderedMap<>();
386+
if (topTerms != null) {
387+
detailedFieldInfo.add(KEY_TOP_TERMS, topTerms);
388+
}
389+
if (fi.getDistinct() > 0) {
390+
detailedFieldInfo.add(KEY_DISTINCT, fi.getDistinct());
391+
}
392+
if (histogram != null) {
393+
detailedFieldInfo.add(KEY_HISTOGRAM, histogram);
394+
}
395+
perShardFields.add(fieldName, detailedFieldInfo);
396+
}
368397
}
369398
}
370399
shardsInfo.add(shardAddr, perShardEntry);
@@ -377,8 +406,12 @@ private void mergeDistributedResponses(SolrQueryResponse rsp, List<ShardResponse
377406
mergedIndex.add(KEY_SEGMENT_COUNT, totalSegmentCount);
378407
rsp.add(RSP_INDEX, mergedIndex);
379408

380-
if (fieldsResult.size() > 0) {
381-
rsp.add(RSP_FIELDS, fieldsResult);
409+
if (!mergedFields.isEmpty()) {
410+
SimpleOrderedMap<Object> mergedFieldsNL = new SimpleOrderedMap<>();
411+
for (Map.Entry<String, SimpleOrderedMap<Object>> entry : mergedFields.entrySet()) {
412+
mergedFieldsNL.add(entry.getKey(), entry.getValue());
413+
}
414+
rsp.add(RSP_FIELDS, mergedFieldsNL);
382415
}
383416

384417
rsp.add(RSP_SHARDS, shardsInfo);
@@ -387,73 +420,70 @@ private void mergeDistributedResponses(SolrQueryResponse rsp, List<ShardResponse
387420
private void mergeShardField(
388421
String shardAddr,
389422
LukeResponse.FieldInfo fi,
390-
String fieldName,
391423
SimpleOrderedMap<Object> merged,
392-
Map<String, FieldOrigin> fieldOrigins,
393-
SimpleOrderedMap<Object> perShardFields,
394-
SimpleOrderedMap<Object> perShardEntry) {
424+
Map<String, ExpectedFieldConfig> expectedFieldConfigs) {
395425

396-
FieldOrigin origin = fieldOrigins.get(fieldName);
426+
String fieldName = fi.getName();
427+
ExpectedFieldConfig origin = expectedFieldConfigs.get(fieldName);
397428
if (origin == null) {
398-
fieldOrigins.put(fieldName, new FieldOrigin(shardAddr, fi));
399-
// First shard: populate merged with schema-derived attrs
429+
origin = new ExpectedFieldConfig(shardAddr, fi);
430+
expectedFieldConfigs.put(fieldName, origin);
431+
// First shard to report this field: populate merged with schema-derived attrs
400432
merged.add(KEY_TYPE, fi.getType());
401433
merged.add(KEY_SCHEMA_FLAGS, fi.getSchema());
402434
Object dynBase = fi.getExtras().get(KEY_DYNAMIC_BASE);
403435
if (dynBase != null) {
404436
merged.add(KEY_DYNAMIC_BASE, dynBase);
405437
}
438+
if (origin.indexFlags != null) {
439+
merged.add(KEY_INDEX_FLAGS, origin.indexFlags);
440+
}
406441
} else {
407-
// Subsequent shards: validate consistency of schema-derived attrs
442+
// Subsequent shards: validate consistency
408443
validateFieldAttr(
409444
fieldName,
410445
KEY_TYPE,
411446
fi.getType(),
412-
origin.fieldInfo().getType(),
447+
origin.fieldInfo.getType(),
413448
shardAddr,
414-
origin.shardAddr());
449+
origin.shardAddr);
415450
validateFieldAttr(
416451
fieldName,
417452
KEY_SCHEMA_FLAGS,
418453
fi.getSchema(),
419-
origin.fieldInfo().getSchema(),
454+
origin.fieldInfo.getSchema(),
420455
shardAddr,
421-
origin.shardAddr());
456+
origin.shardAddr);
422457
validateFieldAttr(
423458
fieldName,
424459
KEY_DYNAMIC_BASE,
425460
fi.getExtras().get(KEY_DYNAMIC_BASE),
426-
origin.fieldInfo().getExtras().get(KEY_DYNAMIC_BASE),
461+
origin.fieldInfo.getExtras().get(KEY_DYNAMIC_BASE),
427462
shardAddr,
428-
origin.shardAddr());
463+
origin.shardAddr);
464+
465+
Object indexFlags = fi.getExtras().get(KEY_INDEX_FLAGS);
466+
if (indexFlags != null) {
467+
if (origin.indexFlags == null) {
468+
origin.indexFlags = indexFlags;
469+
origin.indexFlagsShardAddr = shardAddr;
470+
merged.add(KEY_INDEX_FLAGS, indexFlags);
471+
} else {
472+
validateFieldAttr(
473+
fieldName,
474+
KEY_INDEX_FLAGS,
475+
indexFlags,
476+
origin.indexFlags,
477+
shardAddr,
478+
origin.indexFlagsShardAddr);
479+
}
480+
}
429481
}
430482

431-
// Index flags: take first non-null (index-derived, may differ across shards)
432-
merged.computeIfAbsent(KEY_INDEX_FLAGS, k -> fi.getExtras().get(KEY_INDEX_FLAGS));
433-
434483
Long docsAsLong = fi.getDocsAsLong();
435484
if (docsAsLong != null && docsAsLong > 0) {
436485
merged.compute(
437-
"docsAsLong", (key, val) -> val == null ? docsAsLong : (Long) val + docsAsLong);
438-
}
439-
440-
// Detailed stats — kept per-shard, not merged
441-
NamedList<Integer> topTerms = fi.getTopTerms();
442-
Object histogram = fi.getExtras().get(KEY_HISTOGRAM);
443-
444-
if (topTerms != null || fi.getDistinct() > 0 || histogram != null) {
445-
perShardEntry.putIfAbsent(RSP_FIELDS, perShardFields);
446-
SimpleOrderedMap<Object> detailedFieldInfo = new SimpleOrderedMap<>();
447-
if (topTerms != null) {
448-
detailedFieldInfo.add(KEY_TOP_TERMS, topTerms);
449-
}
450-
if (fi.getDistinct() > 0) {
451-
detailedFieldInfo.add(KEY_DISTINCT, fi.getDistinct());
452-
}
453-
if (histogram != null) {
454-
detailedFieldInfo.add(KEY_HISTOGRAM, histogram);
455-
}
456-
perShardFields.add(fieldName, detailedFieldInfo);
486+
KEY_DOCS_AS_LONG, (key, val) -> val == null ? docsAsLong : (Long) val + docsAsLong);
457487
}
458488
}
459489

‎solr/core/src/test/org/apache/solr/handler/admin/LukeRequestHandlerDistribTest.java‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ public void testDistributedMerge() throws Exception {
108108
}
109109

110110
@Test
111+
@SuppressWarnings("unchecked")
111112
public void testDistributedFieldsMerge() throws Exception {
112113
ModifiableSolrParams params = new ModifiableSolrParams();
113114
params.set("distrib", "true");
@@ -129,6 +130,14 @@ public void testDistributedFieldsMerge() throws Exception {
129130
LukeResponse.FieldInfo idField = fields.get("id");
130131
assertNotNull("'id' field should be present", idField);
131132
assertEquals("id field type should be string", "string", idField.getType());
133+
134+
// Index flags should be consistent across shards (both shards have data for "name").
135+
// The merge validates non-null index flags for consistency; if they were inconsistent,
136+
// the request would have thrown an error. Verify the merged result has index flags.
137+
NamedList<Object> mergedFieldsNL = (NamedList<Object>) rsp.getResponse().get("fields");
138+
NamedList<Object> rawNameField = (NamedList<Object>) mergedFieldsNL.get("name");
139+
assertNotNull(
140+
"index flags should be present when both shards have data", rawNameField.get("index"));
132141
}
133142

134143
@Test
@@ -259,12 +268,14 @@ public void testSparseShards() throws Exception {
259268
assertNotNull("cat_s type", catField.getType());
260269
assertNotNull("cat_s dynamicBase", catField.getExtras().get("dynamicBase"));
261270

262-
// Verify index flags are present (from the one shard that has the document).
263-
// Fields that are indexed and have a live doc should get index flags via the merge's
264-
// computeIfAbsent (take-first-non-null) logic.
265-
NamedList<Object> mergedFields = (NamedList<Object>) rsp.getResponse().get("fields");
266-
assertNotNull("merged fields NamedList should be present", mergedFields);
267-
NamedList<Object> rawNameField = (NamedList<Object>) mergedFields.get("name");
271+
// Verify index flags in the merged response for the static "name" field.
272+
// Luke only reports fields present in the Lucene index (via reader.getFieldInfos()),
273+
// so only the shard with the document contributes "name" to the merge. The merge
274+
// validates consistency of index flags across shards (null is always consistent),
275+
// but with 11 empty shards, only one shard contributes index flags here.
276+
NamedList<Object> mergedFieldsNL = (NamedList<Object>) rsp.getResponse().get("fields");
277+
assertNotNull("merged fields NamedList should be present", mergedFieldsNL);
278+
NamedList<Object> rawNameField = (NamedList<Object>) mergedFieldsNL.get("name");
268279
assertNotNull("raw 'name' field should be in merged fields", rawNameField);
269280
// The index flags key may or may not be present depending on whether the field is indexed
270281
// and stored — but if present, it should be a non-empty string

0 commit comments

Comments
 (0)