dsmiley commented on code in PR #4717:
URL: https://github.com/apache/solr/pull/4717#discussion_r3942558769
##########
solr/core/src/java/org/apache/solr/update/SolrIndexSplitter.java:
##########
@@ -677,6 +682,20 @@ static FixedBitSet[] split(
}
}
+ if (field.getType().isPointField()) {
Review Comment:
From what I see, this is too specific. The reason we need to split off to
do another algorithm is not specifically that this is a "point field". It's
that there is *not* a terms index; the lower code depends on Terms. So if the
router.field isn't "indexed" but it has docValues, then the code you wrote
could be improved to handle that. I'll do that.
##########
solr/core/src/java/org/apache/solr/update/SolrIndexSplitter.java:
##########
@@ -746,38 +765,145 @@ static FixedBitSet[] split(
}
if (docsMatchingRanges != null) {
- for (int ii = 0; ii < docsMatchingRanges.length; ii++) {
- if (0 == docsMatchingRanges[ii]) continue;
- switch (ii) {
- case 0:
- // document loss
- log.error(
- "Splitting {}: {} documents belong to no shards and will be
dropped",
- reader,
- docsMatchingRanges[ii]);
- break;
- case 1:
- // normal case, each document moves to one of the sub-shards
- log.info(
- "Splitting {}: {} documents will move into a sub-shard",
- reader,
- docsMatchingRanges[ii]);
- break;
- default:
- // document duplication
- log.error(
- "Splitting {}: {} documents will be moved to multiple ({})
sub-shards",
- reader,
- docsMatchingRanges[ii],
- ii);
- break;
+ logDocsMatchingRanges(reader, docsMatchingRanges);
+ }
+
+ return docSets;
+ }
+
+ private static FixedBitSet[] splitPointField(
+ LeafReader reader,
+ int numPieces,
+ SchemaField field,
+ DocRouter.Range[] rangesArr,
+ String splitKey,
+ HashBasedRouter hashRouter,
+ boolean delete,
+ FixedBitSet[] docSets,
+ Bits liveDocs,
+ AtomicInteger currentPartition)
+ throws IOException {
+ NumericDocValues numericDocValues =
+ field.hasDocValues() ? DocValues.getNumeric(reader, field.getName()) :
null;
+
+ int[] docsMatchingRanges = null;
+ if (rangesArr != null) {
+ docsMatchingRanges = new int[rangesArr.length + 1];
+ }
+
+ for (int doc = 0; doc < reader.maxDoc(); doc++) {
+ if (liveDocs != null && !liveDocs.get(doc)) {
+ continue;
+ }
+
+ String routeValue = getRouteFieldValue(reader, doc, field,
numericDocValues);
+ if (splitKey != null) {
+ String part1 = ((CompositeIdRouter)
hashRouter).getRouteKeyNoSuffix(routeValue);
+ if (part1 == null || !splitKey.equals(part1)) {
+ continue;
}
}
+
+ if (rangesArr == null) {
+ if (delete) {
+ docSets[currentPartition.get()].clear(doc);
+ } else {
+ docSets[currentPartition.get()].set(doc);
+ }
+ currentPartition.set((currentPartition.get() + 1) % numPieces);
+ } else {
+ int hash = hashRouter.sliceHash(routeValue, null, null, null);
+ int matchingRangesCount = 0;
+ for (int i = 0; i < rangesArr.length; i++) {
+ if (rangesArr[i].includes(hash)) {
+ if (delete) {
+ docSets[i].clear(doc);
+ } else {
+ docSets[i].set(doc);
+ }
+ ++matchingRangesCount;
+ }
+ }
+ docsMatchingRanges[matchingRangesCount]++;
+ }
}
+ if (docsMatchingRanges != null) {
+ logDocsMatchingRanges(reader, docsMatchingRanges);
+ }
return docSets;
}
+ private static String getRouteFieldValue(
+ LeafReader reader, int doc, SchemaField field, NumericDocValues
numericDocValues)
+ throws IOException {
+ if (numericDocValues != null && numericDocValues.advanceExact(doc)) {
+ return numericRouteValueToString(field, numericDocValues.longValue());
+ }
+
+ if (field.stored()) {
+ Document storedDocument = reader.storedFields().document(doc);
+ IndexableField storedField = storedDocument.getField(field.getName());
+ if (storedField != null) {
+ Object routeValue = field.getType().toObject(storedField);
+ if (routeValue != null) {
+ return routeValue.toString();
+ }
+ }
+ }
+
+ throw new SolrException(
+ SolrException.ErrorCode.SERVER_ERROR,
+ "Unable to read route field '"
+ + field.getName()
+ + "' for shard splitting. Point-based route fields must expose
docValues or be stored.");
+ }
+
+ private static String numericRouteValueToString(SchemaField field, long
value) {
+ NumberType numberType = field.getType().getNumberType();
+ if (numberType == null) {
+ return Long.toString(value);
+ }
+
+ return switch (numberType) {
+ case INTEGER -> Integer.toString((int) value);
+ case LONG -> Long.toString(value);
+ case FLOAT -> Float.toString(Float.intBitsToFloat((int) value));
+ case DOUBLE -> Double.toString(Double.longBitsToDouble(value));
+ case DATE -> Long.toString(value);
Review Comment:
I doubt we truly support dates for router.field.
##########
solr/core/src/java/org/apache/solr/update/SolrIndexSplitter.java:
##########
@@ -746,38 +765,145 @@ static FixedBitSet[] split(
}
if (docsMatchingRanges != null) {
- for (int ii = 0; ii < docsMatchingRanges.length; ii++) {
- if (0 == docsMatchingRanges[ii]) continue;
- switch (ii) {
- case 0:
- // document loss
- log.error(
- "Splitting {}: {} documents belong to no shards and will be
dropped",
- reader,
- docsMatchingRanges[ii]);
- break;
- case 1:
- // normal case, each document moves to one of the sub-shards
- log.info(
- "Splitting {}: {} documents will move into a sub-shard",
- reader,
- docsMatchingRanges[ii]);
- break;
- default:
- // document duplication
- log.error(
- "Splitting {}: {} documents will be moved to multiple ({})
sub-shards",
- reader,
- docsMatchingRanges[ii],
- ii);
- break;
+ logDocsMatchingRanges(reader, docsMatchingRanges);
+ }
+
+ return docSets;
+ }
+
+ private static FixedBitSet[] splitPointField(
+ LeafReader reader,
+ int numPieces,
+ SchemaField field,
+ DocRouter.Range[] rangesArr,
+ String splitKey,
+ HashBasedRouter hashRouter,
+ boolean delete,
+ FixedBitSet[] docSets,
+ Bits liveDocs,
+ AtomicInteger currentPartition)
+ throws IOException {
+ NumericDocValues numericDocValues =
+ field.hasDocValues() ? DocValues.getNumeric(reader, field.getName()) :
null;
+
+ int[] docsMatchingRanges = null;
+ if (rangesArr != null) {
+ docsMatchingRanges = new int[rangesArr.length + 1];
+ }
+
+ for (int doc = 0; doc < reader.maxDoc(); doc++) {
+ if (liveDocs != null && !liveDocs.get(doc)) {
+ continue;
+ }
+
+ String routeValue = getRouteFieldValue(reader, doc, field,
numericDocValues);
+ if (splitKey != null) {
+ String part1 = ((CompositeIdRouter)
hashRouter).getRouteKeyNoSuffix(routeValue);
+ if (part1 == null || !splitKey.equals(part1)) {
+ continue;
}
}
+
+ if (rangesArr == null) {
+ if (delete) {
+ docSets[currentPartition.get()].clear(doc);
+ } else {
+ docSets[currentPartition.get()].set(doc);
+ }
+ currentPartition.set((currentPartition.get() + 1) % numPieces);
+ } else {
+ int hash = hashRouter.sliceHash(routeValue, null, null, null);
+ int matchingRangesCount = 0;
+ for (int i = 0; i < rangesArr.length; i++) {
+ if (rangesArr[i].includes(hash)) {
+ if (delete) {
+ docSets[i].clear(doc);
+ } else {
+ docSets[i].set(doc);
+ }
+ ++matchingRangesCount;
+ }
+ }
+ docsMatchingRanges[matchingRangesCount]++;
+ }
}
+ if (docsMatchingRanges != null) {
+ logDocsMatchingRanges(reader, docsMatchingRanges);
+ }
return docSets;
}
+ private static String getRouteFieldValue(
+ LeafReader reader, int doc, SchemaField field, NumericDocValues
numericDocValues)
+ throws IOException {
+ if (numericDocValues != null && numericDocValues.advanceExact(doc)) {
+ return numericRouteValueToString(field, numericDocValues.longValue());
+ }
+
+ if (field.stored()) {
+ Document storedDocument = reader.storedFields().document(doc);
+ IndexableField storedField = storedDocument.getField(field.getName());
+ if (storedField != null) {
+ Object routeValue = field.getType().toObject(storedField);
+ if (routeValue != null) {
+ return routeValue.toString();
+ }
+ }
+ }
+
+ throw new SolrException(
+ SolrException.ErrorCode.SERVER_ERROR,
+ "Unable to read route field '"
+ + field.getName()
+ + "' for shard splitting. Point-based route fields must expose
docValues or be stored.");
+ }
+
+ private static String numericRouteValueToString(SchemaField field, long
value) {
+ NumberType numberType = field.getType().getNumberType();
+ if (numberType == null) {
+ return Long.toString(value);
+ }
+
+ return switch (numberType) {
Review Comment:
This can be less low-level.. and thus less worry about the specific types.
I'll contribute an improvement.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
