You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2020/12/21 21:46:50 UTC
[GitHub] [lucene-solr] dsmiley opened a new pull request #2159: SOLR-14923: Nested docs indexing performance.
dsmiley opened a new pull request #2159:
URL: https://github.com/apache/lucene-solr/pull/2159
https://issues.apache.org/jira/browse/SOLR-14923
* UpdateLog.openRealtimeSearcher() was being called for every incoming document when the schema had _nest_path_. It is now more limited to in-place-update of a child doc and in /get when a child doc is given, both of which are uncommon.
* Atomic/partial updates to nested documents should be faster. In-place updates of the same might be slower (needs to call openRealtimeSearcher).
Refactoring & minor bugs/improvements:
* Simplified AddUpdateCommand.getLuceneDoc & getLuceneDocsIfNested relationship, and DirectUpdateHandler2 which calls them.
* AddUpdateCommand.getIndexedId needed to be updated when _route_ was specified.
* NestedShardedAtomicUpdateTest no longer extends AbstractFullDistribZkTestBase because it wasn't really leveraging the "control client" checking, and it added too much complexity to debug failures.
* AtomicUpdateDocumentMerger: simplified merge; possibly now supports updates to anonymous children
* No longer need RTG.Resolution.DOC_WITH_CHILDREN
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dsmiley merged pull request #2159: SOLR-14923: Nested docs indexing performance.
Posted by GitBox <gi...@apache.org>.
dsmiley merged pull request #2159:
URL: https://github.com/apache/lucene-solr/pull/2159
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] muse-dev[bot] commented on a change in pull request #2159: SOLR-14923: Nested docs indexing performance.
Posted by GitBox <gi...@apache.org>.
muse-dev[bot] commented on a change in pull request #2159:
URL: https://github.com/apache/lucene-solr/pull/2159#discussion_r551131159
##########
File path: solr/core/src/java/org/apache/solr/update/AddUpdateCommand.java
##########
@@ -95,121 +124,129 @@ public SolrInputDocument getSolrInputDocument() {
}
/**
- * Creates and returns a lucene Document to index.
- * Nested documents, if found, will cause an exception to be thrown. Call {@link #getLuceneDocsIfNested()} for that.
+ * Creates and returns a lucene Document for in-place update.
+ * The SolrInputDocument itself may be modified, which will be reflected in the update log.
* Any changes made to the returned Document will not be reflected in the SolrInputDocument, or future calls to this
* method.
- * Note that the behavior of this is sensitive to {@link #isInPlaceUpdate()}.*/
- public Document getLuceneDocument() {
- final boolean ignoreNestedDocs = false; // throw an exception if found
- SolrInputDocument solrInputDocument = getSolrInputDocument();
- if (!isInPlaceUpdate() && getReq().getSchema().isUsableForChildDocs()) {
- addRootField(solrInputDocument, getRootIdUsingRouteParam());
- }
- return DocumentBuilder.toDocument(solrInputDocument, req.getSchema(), isInPlaceUpdate(), ignoreNestedDocs);
- }
-
- /** Returns the indexed ID for this document. The returned BytesRef is retained across multiple calls, and should not be modified. */
- public BytesRef getIndexedId() {
- if (indexedId == null) {
- IndexSchema schema = req.getSchema();
- SchemaField sf = schema.getUniqueKeyField();
- if (sf != null) {
- if (solrDoc != null) {
- SolrInputField field = solrDoc.getField(sf.getName());
-
- int count = field==null ? 0 : field.getValueCount();
- if (count == 0) {
- if (overwrite) {
- throw new SolrException( SolrException.ErrorCode.BAD_REQUEST,"Document is missing mandatory uniqueKey field: " + sf.getName());
- }
- } else if (count > 1) {
- throw new SolrException( SolrException.ErrorCode.BAD_REQUEST,"Document contains multiple values for uniqueKey field: " + field);
- } else {
- BytesRefBuilder b = new BytesRefBuilder();
- sf.getType().readableToIndexed(field.getFirstValue().toString(), b);
- indexedId = b.get();
- }
- }
- }
+ */
+ Document makeLuceneDocForInPlaceUpdate() {
+ // perhaps this should move to UpdateHandler or DocumentBuilder?
+ assert isInPlaceUpdate();
+ if (req.getSchema().isUsableForChildDocs() && solrDoc.getField(IndexSchema.ROOT_FIELD_NAME) == null) {
+ solrDoc.setField(IndexSchema.ROOT_FIELD_NAME, getIndexedIdStr());
}
- return indexedId;
- }
-
- public void setIndexedId(BytesRef indexedId) {
- this.indexedId = indexedId;
+ final boolean forInPlaceUpdate = true;
+ final boolean ignoreNestedDocs = false; // throw an exception if found
+ return DocumentBuilder.toDocument(solrDoc, req.getSchema(), forInPlaceUpdate, ignoreNestedDocs);
}
- public String getPrintableId() {
- if (req != null) {
- IndexSchema schema = req.getSchema();
- SchemaField sf = schema.getUniqueKeyField();
- if (solrDoc != null && sf != null) {
- SolrInputField field = solrDoc.getField(sf.getName());
- if (field != null) {
- return field.getFirstValue().toString();
- }
- }
- }
- return "(null)";
- }
+ /**
+ * Returns the indexed ID for this document, or the root ID for nested documents.
+ *
+ * @return possibly null if there's no uniqueKey field
+ */
+ public String getIndexedIdStr() {
+ extractIdsIfNeeded();
+ return indexedIdStr;
+ }
/**
+ * Returns the indexed ID for this document, or the root ID for nested documents. The returned
+ * BytesRef should be treated as immutable. It will not be re-used/modified for additional docs.
*
- * @return value of _route_ param({@link ShardParams#_ROUTE_}), otherwise doc id.
+ * @return possibly null if there's no uniqueKey field
*/
- public String getRootIdUsingRouteParam() {
- return req.getParams().get(ShardParams._ROUTE_, getHashableId());
- }
+ public BytesRef getIndexedId() {
+ extractIdsIfNeeded();
+ return indexedId;
+ }
/**
- * @return String id to hash
+ * Returns the ID of the doc itself, possibly different from {@link #getIndexedIdStr()} which
+ * points to the root doc.
+ *
+ * @return possibly null if there's no uniqueKey field
*/
- public String getHashableId() {
+ public String getChildDocIdStr() {
+ extractIdsIfNeeded();
+ return childDocIdStr;
+ }
+
+ /** The ID for logging purposes. */
+ public String getPrintableId() {
+ extractIdsIfNeeded();
+ if (indexedIdStr == null) {
+ return "(null)";
+ } else if (indexedIdStr.equals(childDocIdStr)) {
+ return indexedIdStr;
+ } else {
+ return childDocIdStr + " (root=" + indexedIdStr + ")";
+ }
+ }
+
+ private void extractIdsIfNeeded() {
+ if (indexedId != null) {
+ return;
+ }
IndexSchema schema = req.getSchema();
SchemaField sf = schema.getUniqueKeyField();
if (sf != null) {
if (solrDoc != null) {
SolrInputField field = solrDoc.getField(sf.getName());
-
- int count = field == null ? 0 : field.getValueCount();
+ // check some uniqueKey constraints
+ int count = field==null ? 0 : field.getValueCount();
if (count == 0) {
if (overwrite) {
- throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
- "Document is missing mandatory uniqueKey field: "
- + sf.getName());
+ throw new SolrException( SolrException.ErrorCode.BAD_REQUEST,"Document is missing mandatory uniqueKey field: " + sf.getName());
}
- } else if (count > 1) {
- throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
- "Document contains multiple values for uniqueKey field: " + field);
+ } else if (count > 1) {
+ throw new SolrException( SolrException.ErrorCode.BAD_REQUEST,"Document contains multiple values for uniqueKey field: " + field);
} else {
- return field.getFirstValue().toString();
+ this.childDocIdStr = field.getFirstValue().toString();
Review comment:
*NULL_DEREFERENCE:* object returned by `field.getFirstValue()` could be null and is dereferenced at line 205.
##########
File path: solr/core/src/java/org/apache/solr/update/UpdateLog.java
##########
@@ -561,6 +565,13 @@ public void add(AddUpdateCommand cmd, boolean clearCaches) {
// TODO: we currently need to log to maintain correct versioning, rtg, etc
// if ((cmd.getFlags() & UpdateCommand.REPLAY) != 0) return;
+ // This hack could be removed after SOLR-15064 when we insist updates to child docs include _root_.
+ // Until then, if we're in a buffering mode, then the solrDoc won't have the _root_ field.
+ // Otherwise, it should already be there, placed by the client.
+ if (usableForChildDocs && cmd.useRouteAsRoot != null && cmd.solrDoc.getField(IndexSchema.ROOT_FIELD_NAME) == null) {
Review comment:
*THREAD_SAFETY_VIOLATION:* Read/Write race. Non-private method `UpdateLog.add(...)` indirectly reads without synchronization from container `cmd.solrDoc._fields` via call to `Map.get(...)`. Potentially races with write in method `UpdateLog.add(...)`.
Reporting because this access may occur on a background thread.
##########
File path: solr/core/src/java/org/apache/solr/update/UpdateLog.java
##########
@@ -561,6 +565,13 @@ public void add(AddUpdateCommand cmd, boolean clearCaches) {
// TODO: we currently need to log to maintain correct versioning, rtg, etc
// if ((cmd.getFlags() & UpdateCommand.REPLAY) != 0) return;
+ // This hack could be removed after SOLR-15064 when we insist updates to child docs include _root_.
+ // Until then, if we're in a buffering mode, then the solrDoc won't have the _root_ field.
+ // Otherwise, it should already be there, placed by the client.
+ if (usableForChildDocs && cmd.useRouteAsRoot != null && cmd.solrDoc.getField(IndexSchema.ROOT_FIELD_NAME) == null) {
+ cmd.solrDoc.setField(IndexSchema.ROOT_FIELD_NAME, cmd.getIndexedIdStr());
Review comment:
*THREAD_SAFETY_VIOLATION:* Unprotected write. Non-private method `UpdateLog.add(...)` indirectly writes to field `cmd.childDocIdStr` outside of synchronization.
Reporting because this access may occur on a background thread.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] ctargett commented on a change in pull request #2159: SOLR-14923: Nested docs indexing performance.
Posted by GitBox <gi...@apache.org>.
ctargett commented on a change in pull request #2159:
URL: https://github.com/apache/lucene-solr/pull/2159#discussion_r552851326
##########
File path: solr/solr-ref-guide/src/indexing-nested-documents.adoc
##########
@@ -218,31 +225,40 @@ Indexing nested documents _requires_ an indexed field named `\_root_`:
[source,xml]
----
-<field name="_root_" type="string" indexed="true" />
+<field name="_root_" type="string" indexed="true" stored="false" docValues="false" />
----
-Solr automatically populates this field in every nested document with the `id` value of the top most parent document in the block.
+Solr automatically populates this field in _all_ documents with the `id` value of it's root document
+-- it's highest ancestor, possibly itself.
There are several additional schema considerations that should be considered for people who wish to use nested documents:
-* Nested child documents are very much documents in their own right even if certain nested documents hold different information from the parent, Therefore:
+* Nested child documents are very much documents in their own right even if certain nested
+documents hold different information from the parent or other child documents, therefore:
** All field names in the schema can only be configured in one -- different types of child documents can not have the same field name configured in different ways.
-** It may be infeasible to use `required` for any field names that aren't reqiured for all types of documents.
+** It may be infeasible to use `required` for any field names that aren't required for all types of
+documents.
** Even child documents need a _globally_ unique `id`.
-* `\_root_` must be configured to either be stored (`stored="true"`) or use doc values (`docValues="true"`) to enable <<updating-parts-of-documents#updating-child-documents,atomic updates of nested documents>>.
-** Also, beware of `uniqueBlock(\_root_)` <<json-facet-api#stat-facet-functions,field type limitation>>, if you plan to use one.
-* `\_nest_path_` is an optional field that (if definied) will be populated by Solr automatically with the ancestor path of each non-root document.
+* The `\_root_` must be indexed (`indexed="true"`) but doesn't need to
+ be either stored (`stored="true"`) or use doc values (`docValues="true"`), however you are free
+to do so if you find it useful. If you want to use `uniqueBlock(\_root_)`
+<<json-facet-api#stat-facet-functions,field type limitation>>, then you should enable docValues.
Review comment:
The removed section introduces the `_nest_path_` field which is shown in the example that has been retained. However the change here talks about the field requirements for the `_root_` field. That leaves the `_nest_path_` field unaddressed and makes the following example seem like it's coming out of nowhere. This gets a bit more confusing as the section goes on, with a later sub-bullet that talks about the `_nest_path_` field which hasn't been discussed really up to that point (the example is literally the first time in the doc that it's mentioned).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dsmiley commented on pull request #2159: SOLR-14923: Nested docs indexing performance.
Posted by GitBox <gi...@apache.org>.
dsmiley commented on pull request #2159:
URL: https://github.com/apache/lucene-solr/pull/2159#issuecomment-749216411
CC @thomaswoeckinger @moshebla
Tests pass, precommit passes.
@markrmiller FYI I observed that NestedShardedAtomicUpdateTest on your ref-impl branch commented out an assertion that I wound up fixing in this PR. I converted that test to use SolrCloudTestCase which made it way easier to debug some elusive bugs.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dsmiley commented on pull request #2159: SOLR-14923: Nested docs indexing performance.
Posted by GitBox <gi...@apache.org>.
dsmiley commented on pull request #2159:
URL: https://github.com/apache/lucene-solr/pull/2159#issuecomment-756205717
I quashed the changes, rebased on top of master, added a commit of some minor things, and force pushed.
I plan to commit tonight.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] muse-dev[bot] commented on a change in pull request #2159: SOLR-14923: Nested docs indexing performance.
Posted by GitBox <gi...@apache.org>.
muse-dev[bot] commented on a change in pull request #2159:
URL: https://github.com/apache/lucene-solr/pull/2159#discussion_r552231540
##########
File path: solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformer.java
##########
@@ -76,6 +87,37 @@ public String getName() {
@Override
public boolean needsSolrIndexSearcher() { return true; }
+ @Override
+ public String[] getExtraRequestFields() {
+ return extraRequestedFields;
+ }
+
+ private int getPrevRootGivenFilter(LeafReaderContext leafReaderContext, int segRootId) throws IOException {
+ final BitSet segParentsBitSet = parentsFilter.getBitSet(leafReaderContext);
+ if (segParentsBitSet != null) {
+ return segRootId == 0 ? -1 : segParentsBitSet.prevSetBit(segRootId - 1);
+ }
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
+ "Parent filter '" + parentsFilter + "' doesn't match any parent documents");
+ }
+
+ private int getPrevRootGivenId(LeafReaderContext leafReaderContext, int segRootId,
+ BytesRef idBytes) throws IOException {
+ final LeafReader reader = leafReaderContext.reader();
+ final Terms terms = reader.terms(IndexSchema.ROOT_FIELD_NAME); // never returns null here
+ final TermsEnum iterator = terms.iterator();
+ if (iterator.seekExact(idBytes)) {
+ PostingsEnum docs = iterator.postings(null, PostingsEnum.NONE);
+ docs = BitsFilteredPostingsEnum.wrap(docs, reader.getLiveDocs());
+ int id = docs.nextDoc();
Review comment:
*NULL_DEREFERENCE:* object `docs` last assigned on line 111 could be null and is dereferenced at line 112.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #2159: SOLR-14923: Nested docs indexing performance.
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #2159:
URL: https://github.com/apache/lucene-solr/pull/2159#discussion_r552241596
##########
File path: solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformer.java
##########
@@ -76,6 +87,37 @@ public String getName() {
@Override
public boolean needsSolrIndexSearcher() { return true; }
+ @Override
+ public String[] getExtraRequestFields() {
+ return extraRequestedFields;
+ }
+
+ private int getPrevRootGivenFilter(LeafReaderContext leafReaderContext, int segRootId) throws IOException {
+ final BitSet segParentsBitSet = parentsFilter.getBitSet(leafReaderContext);
+ if (segParentsBitSet != null) {
+ return segRootId == 0 ? -1 : segParentsBitSet.prevSetBit(segRootId - 1);
+ }
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
+ "Parent filter '" + parentsFilter + "' doesn't match any parent documents");
+ }
+
+ private int getPrevRootGivenId(LeafReaderContext leafReaderContext, int segRootId,
+ BytesRef idBytes) throws IOException {
+ final LeafReader reader = leafReaderContext.reader();
+ final Terms terms = reader.terms(IndexSchema.ROOT_FIELD_NAME); // never returns null here
+ final TermsEnum iterator = terms.iterator();
+ if (iterator.seekExact(idBytes)) {
+ PostingsEnum docs = iterator.postings(null, PostingsEnum.NONE);
+ docs = BitsFilteredPostingsEnum.wrap(docs, reader.getLiveDocs());
+ int id = docs.nextDoc();
Review comment:
Wrong; iterator.postings() never returns null -- so says its documentation.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #2159: SOLR-14923: Nested docs indexing performance.
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #2159:
URL: https://github.com/apache/lucene-solr/pull/2159#discussion_r552920830
##########
File path: solr/solr-ref-guide/src/indexing-nested-documents.adoc
##########
@@ -218,31 +225,40 @@ Indexing nested documents _requires_ an indexed field named `\_root_`:
[source,xml]
----
-<field name="_root_" type="string" indexed="true" />
+<field name="_root_" type="string" indexed="true" stored="false" docValues="false" />
----
-Solr automatically populates this field in every nested document with the `id` value of the top most parent document in the block.
+Solr automatically populates this field in _all_ documents with the `id` value of it's root document
+-- it's highest ancestor, possibly itself.
There are several additional schema considerations that should be considered for people who wish to use nested documents:
-* Nested child documents are very much documents in their own right even if certain nested documents hold different information from the parent, Therefore:
+* Nested child documents are very much documents in their own right even if certain nested
+documents hold different information from the parent or other child documents, therefore:
** All field names in the schema can only be configured in one -- different types of child documents can not have the same field name configured in different ways.
-** It may be infeasible to use `required` for any field names that aren't reqiured for all types of documents.
+** It may be infeasible to use `required` for any field names that aren't required for all types of
+documents.
** Even child documents need a _globally_ unique `id`.
-* `\_root_` must be configured to either be stored (`stored="true"`) or use doc values (`docValues="true"`) to enable <<updating-parts-of-documents#updating-child-documents,atomic updates of nested documents>>.
-** Also, beware of `uniqueBlock(\_root_)` <<json-facet-api#stat-facet-functions,field type limitation>>, if you plan to use one.
-* `\_nest_path_` is an optional field that (if definied) will be populated by Solr automatically with the ancestor path of each non-root document.
+* The `\_root_` must be indexed (`indexed="true"`) but doesn't need to
+ be either stored (`stored="true"`) or use doc values (`docValues="true"`), however you are free
+to do so if you find it useful. If you want to use `uniqueBlock(\_root_)`
+<<json-facet-api#stat-facet-functions,field type limitation>>, then you should enable docValues.
Review comment:
I don't think I broke the flow of introduction. Perhaps the diff makes this unclear? Any way, I just submitted an update that improves the whole matter (I hope). I recommend building the HTML file looking at it in a browser.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dsmiley commented on pull request #2159: SOLR-14923: Nested docs indexing performance.
Posted by GitBox <gi...@apache.org>.
dsmiley commented on pull request #2159:
URL: https://github.com/apache/lucene-solr/pull/2159#issuecomment-754901726
I believe everything MUSE_DEV reported is a non-issue. Only one thread touches any AddUpdateCommand and the SolrInputDoc on it. Perhaps MUSE_DEV saw the `synchronized` keyword inside UpdateLog.add and thought otherwise. But that thing is synchronizing on UpdateLog itself, not the command.
I included SOLR-15069 (parentFilter on [child] is now not needed) inside here, but that should get merged first, and thus it won't show up ultimately for this issue. SOLR-15069 allows us to safely (no loss of data) do atomic/partial updates when there is no nest path field declared.
@ctargett perhaps you might look over the ref guide changes in here? I did some editing to reduce the use of the word "block" in favor of the word "nest", and also did some other clarifications.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] ctargett commented on a change in pull request #2159: SOLR-14923: Nested docs indexing performance.
Posted by GitBox <gi...@apache.org>.
ctargett commented on a change in pull request #2159:
URL: https://github.com/apache/lucene-solr/pull/2159#discussion_r552983347
##########
File path: solr/solr-ref-guide/src/indexing-nested-documents.adoc
##########
@@ -218,31 +225,40 @@ Indexing nested documents _requires_ an indexed field named `\_root_`:
[source,xml]
----
-<field name="_root_" type="string" indexed="true" />
+<field name="_root_" type="string" indexed="true" stored="false" docValues="false" />
----
-Solr automatically populates this field in every nested document with the `id` value of the top most parent document in the block.
+Solr automatically populates this field in _all_ documents with the `id` value of it's root document
+-- it's highest ancestor, possibly itself.
There are several additional schema considerations that should be considered for people who wish to use nested documents:
-* Nested child documents are very much documents in their own right even if certain nested documents hold different information from the parent, Therefore:
+* Nested child documents are very much documents in their own right even if certain nested
+documents hold different information from the parent or other child documents, therefore:
** All field names in the schema can only be configured in one -- different types of child documents can not have the same field name configured in different ways.
-** It may be infeasible to use `required` for any field names that aren't reqiured for all types of documents.
+** It may be infeasible to use `required` for any field names that aren't required for all types of
+documents.
** Even child documents need a _globally_ unique `id`.
-* `\_root_` must be configured to either be stored (`stored="true"`) or use doc values (`docValues="true"`) to enable <<updating-parts-of-documents#updating-child-documents,atomic updates of nested documents>>.
-** Also, beware of `uniqueBlock(\_root_)` <<json-facet-api#stat-facet-functions,field type limitation>>, if you plan to use one.
-* `\_nest_path_` is an optional field that (if definied) will be populated by Solr automatically with the ancestor path of each non-root document.
+* The `\_root_` must be indexed (`indexed="true"`) but doesn't need to
+ be either stored (`stored="true"`) or use doc values (`docValues="true"`), however you are free
+to do so if you find it useful. If you want to use `uniqueBlock(\_root_)`
+<<json-facet-api#stat-facet-functions,field type limitation>>, then you should enable docValues.
Review comment:
GitHub also allows one to view the whole file as of the commit, which is what I did so I feel like I did already look at your changes in their broader context. I think you've misunderstood the point of my comment, which is not only about that one change, but how it relates to the child bullets under it (which you didn't change, so maybe overlooked).
The bullet point in question talks about the `_route_` field.
Right under that, indented so it is clearly _part of the same bullet item_, is an example from a schema for the `_nest_path_` field. There is no `_route_` field shown in the example. This example used to have text which introduced the `_nest_path_` field, but that was removed to add the current paragraph about the `_route_` field.
This example is now the very first time the `_nest_path_` field is mentioned on the page. What is that field? How does it relate to the `_route_` field?
Right after this example, there is a child-level bullet that states: "In a nutshell, this fields makes child/nested documents easier to use, and so it’s recommended you add it." Which field? The typo makes it even more confusing - does it mean both fields?
The next child-level bullet again says "This field enables Solr to ..." - again, which field?
A later child-level bullet then says, "If you don't use `_nest_path_` ..." This is only the 2nd time this field is mentioned. What is this field? Why would I use it or not use it?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] ctargett commented on pull request #2159: SOLR-14923: Nested docs indexing performance.
Posted by GitBox <gi...@apache.org>.
ctargett commented on pull request #2159:
URL: https://github.com/apache/lucene-solr/pull/2159#issuecomment-755454728
Other than the one section in docs that I commented on, the rest of the doc changes look good - thanks!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #2159: SOLR-14923: Nested docs indexing performance.
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #2159:
URL: https://github.com/apache/lucene-solr/pull/2159#discussion_r552999880
##########
File path: solr/solr-ref-guide/src/indexing-nested-documents.adoc
##########
@@ -218,31 +225,40 @@ Indexing nested documents _requires_ an indexed field named `\_root_`:
[source,xml]
----
-<field name="_root_" type="string" indexed="true" />
+<field name="_root_" type="string" indexed="true" stored="false" docValues="false" />
----
-Solr automatically populates this field in every nested document with the `id` value of the top most parent document in the block.
+Solr automatically populates this field in _all_ documents with the `id` value of it's root document
+-- it's highest ancestor, possibly itself.
There are several additional schema considerations that should be considered for people who wish to use nested documents:
-* Nested child documents are very much documents in their own right even if certain nested documents hold different information from the parent, Therefore:
+* Nested child documents are very much documents in their own right even if certain nested
+documents hold different information from the parent or other child documents, therefore:
** All field names in the schema can only be configured in one -- different types of child documents can not have the same field name configured in different ways.
-** It may be infeasible to use `required` for any field names that aren't reqiured for all types of documents.
+** It may be infeasible to use `required` for any field names that aren't required for all types of
+documents.
** Even child documents need a _globally_ unique `id`.
-* `\_root_` must be configured to either be stored (`stored="true"`) or use doc values (`docValues="true"`) to enable <<updating-parts-of-documents#updating-child-documents,atomic updates of nested documents>>.
-** Also, beware of `uniqueBlock(\_root_)` <<json-facet-api#stat-facet-functions,field type limitation>>, if you plan to use one.
-* `\_nest_path_` is an optional field that (if definied) will be populated by Solr automatically with the ancestor path of each non-root document.
+* The `\_root_` must be indexed (`indexed="true"`) but doesn't need to
+ be either stored (`stored="true"`) or use doc values (`docValues="true"`), however you are free
+to do so if you find it useful. If you want to use `uniqueBlock(\_root_)`
+<<json-facet-api#stat-facet-functions,field type limitation>>, then you should enable docValues.
Review comment:
Thank you for entertaining my request to do a screen share. We're cleared up now; you were looking at my earlier commit, it seems.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org