You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@stormcrawler.apache.org by "mvolikas (via GitHub)" <gi...@apache.org> on 2024/06/15 13:07:12 UTC
[PR] #626: Add routing field in metadata - Solr StatusUpdaterBolt [incubator-stormcrawler]
mvolikas opened a new pull request, #1242:
URL: https://github.com/apache/incubator-stormcrawler/pull/1242
#626
--
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: dev-unsubscribe@stormcrawler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] #626: Add routing field in metadata - Solr StatusUpdaterBolt [incubator-stormcrawler]
Posted by "mvolikas (via GitHub)" <gi...@apache.org>.
mvolikas commented on PR #1242:
URL: https://github.com/apache/incubator-stormcrawler/pull/1242#issuecomment-2250307570
Yes, this variable is #620 related and was unused. I removed it for this PR.
--
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: dev-unsubscribe@stormcrawler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] #626: Add routing field in metadata - Solr StatusUpdaterBolt [incubator-stormcrawler]
Posted by "jnioche (via GitHub)" <gi...@apache.org>.
jnioche commented on PR #1242:
URL: https://github.com/apache/incubator-stormcrawler/pull/1242#issuecomment-2248292777
Thanks @mvolikas
Ok so the logic will be done in #620 so for this one we just need to remove SolrStatusRoutingParamName, correct?
--
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: dev-unsubscribe@stormcrawler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] #626: Add routing field in metadata - Solr StatusUpdaterBolt [incubator-stormcrawler]
Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 commented on PR #1242:
URL: https://github.com/apache/incubator-stormcrawler/pull/1242#issuecomment-2191678579
> do we need _doRouting_ at all? Not sure how the routing is done for SOLR, is it via the collection definition or the indexing API as in OpenSearch?
At least the variable is never used, so might be dropped.
--
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: dev-unsubscribe@stormcrawler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] #626: Add routing field in metadata - Solr StatusUpdaterBolt [incubator-stormcrawler]
Posted by "jnioche (via GitHub)" <gi...@apache.org>.
jnioche commented on PR #1242:
URL: https://github.com/apache/incubator-stormcrawler/pull/1242#issuecomment-2250970146
Thanks @mvolikas
--
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: dev-unsubscribe@stormcrawler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] #626: Add routing field in metadata - Solr StatusUpdaterBolt [incubator-stormcrawler]
Posted by "jnioche (via GitHub)" <gi...@apache.org>.
jnioche merged PR #1242:
URL: https://github.com/apache/incubator-stormcrawler/pull/1242
--
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: dev-unsubscribe@stormcrawler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] #626: Add routing field in metadata - Solr StatusUpdaterBolt [incubator-stormcrawler]
Posted by "mvolikas (via GitHub)" <gi...@apache.org>.
mvolikas commented on code in PR #1242:
URL: https://github.com/apache/incubator-stormcrawler/pull/1242#discussion_r1650031441
##########
external/solr/cores/status/conf/schema.xml:
##########
@@ -25,5 +25,7 @@ under the License.
<field name="status" type="string" indexed="true" stored="true" required="true"/>
<field name="nextFetchDate" type="pdate" stored="true" indexed="true"/>
<dynamicField name="metadata*" type="string" indexed="true" stored="true" multiValued="true"/>
+ <field name="key" type="string" stored="true" indexed="true"/>
Review Comment:
Yes, that's the idea behind the dynamic field, and you are right, "key" should have `multiValued = false` too. thanks for pointing it out!
--
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: dev-unsubscribe@stormcrawler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] #626: Add routing field in metadata - Solr StatusUpdaterBolt [incubator-stormcrawler]
Posted by "jnioche (via GitHub)" <gi...@apache.org>.
jnioche commented on PR #1242:
URL: https://github.com/apache/incubator-stormcrawler/pull/1242#issuecomment-2210995521
> > and point it to the key field, otherwise it will take the unique document key. Is that correct?
>
> Yes, you are right - thanks for noting!
where should `router.field=key` be specified? in solrconfig.xml?
--
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: dev-unsubscribe@stormcrawler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] #626: Add routing field in metadata - Solr StatusUpdaterBolt [incubator-stormcrawler]
Posted by "mvolikas (via GitHub)" <gi...@apache.org>.
mvolikas commented on code in PR #1242:
URL: https://github.com/apache/incubator-stormcrawler/pull/1242#discussion_r1650032704
##########
external/solr/src/main/java/org/apache/stormcrawler/solr/persistence/StatusUpdaterBolt.java:
##########
@@ -83,6 +126,23 @@ public void store(
doc.setField(String.format(Locale.ROOT, "%s.%s", mdPrefix, key), values);
}
+ String partitionKey = partitioner.getPartition(url, metadata);
+ if (partitionKey == null) {
+ partitionKey = "_DEFAULT_";
+ }
+
+ // store routing key in metadata?
+ if (StringUtils.isNotBlank(fieldNameForRoutingKey) && routingFieldNameInMetadata) {
Review Comment:
Yes, i will change this. I followed the logic from opensearch StatusUpdaterBolt that does this check inside `store()` but I guess this improvement can also be applied there.
--
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: dev-unsubscribe@stormcrawler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] #626: Add routing field in metadata - Solr StatusUpdaterBolt [incubator-stormcrawler]
Posted by "mvolikas (via GitHub)" <gi...@apache.org>.
mvolikas commented on code in PR #1242:
URL: https://github.com/apache/incubator-stormcrawler/pull/1242#discussion_r1650032704
##########
external/solr/src/main/java/org/apache/stormcrawler/solr/persistence/StatusUpdaterBolt.java:
##########
@@ -83,6 +126,23 @@ public void store(
doc.setField(String.format(Locale.ROOT, "%s.%s", mdPrefix, key), values);
}
+ String partitionKey = partitioner.getPartition(url, metadata);
+ if (partitionKey == null) {
+ partitionKey = "_DEFAULT_";
+ }
+
+ // store routing key in metadata?
+ if (StringUtils.isNotBlank(fieldNameForRoutingKey) && routingFieldNameInMetadata) {
Review Comment:
Yes, I will change this.
--
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: dev-unsubscribe@stormcrawler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] #626: Add routing field in metadata - Solr StatusUpdaterBolt [incubator-stormcrawler]
Posted by "jnioche (via GitHub)" <gi...@apache.org>.
jnioche commented on code in PR #1242:
URL: https://github.com/apache/incubator-stormcrawler/pull/1242#discussion_r1642425446
##########
external/solr/cores/status/conf/schema.xml:
##########
@@ -25,5 +25,7 @@ under the License.
<field name="status" type="string" indexed="true" stored="true" required="true"/>
<field name="nextFetchDate" type="pdate" stored="true" indexed="true"/>
<dynamicField name="metadata*" type="string" indexed="true" stored="true" multiValued="true"/>
+ <field name="key" type="string" stored="true" indexed="true"/>
Review Comment:
@mvolikas why not set the key field to multiValued = false?
I assume the dynamic field is to allow users to set a different name than 'key'
##########
external/solr/src/main/java/org/apache/stormcrawler/solr/persistence/StatusUpdaterBolt.java:
##########
@@ -83,6 +126,23 @@ public void store(
doc.setField(String.format(Locale.ROOT, "%s.%s", mdPrefix, key), values);
}
+ String partitionKey = partitioner.getPartition(url, metadata);
+ if (partitionKey == null) {
+ partitionKey = "_DEFAULT_";
+ }
+
+ // store routing key in metadata?
+ if (StringUtils.isNotBlank(fieldNameForRoutingKey) && routingFieldNameInMetadata) {
Review Comment:
given that this will not change from one document to the other, why not determine the field name to use once and for all instead of doing it for every document?
--
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: dev-unsubscribe@stormcrawler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] #626: Add routing field in metadata - Solr StatusUpdaterBolt [incubator-stormcrawler]
Posted by "mvolikas (via GitHub)" <gi...@apache.org>.
mvolikas commented on PR #1242:
URL: https://github.com/apache/incubator-stormcrawler/pull/1242#issuecomment-2248170953
Thanks for the patience - I just found some time to look again into this :-)
> where should router.field=key be specified? in solrconfig.xml?
I don't think this will work. The `router.field` is configured during the collection creation with the [Collections API](https://solr.apache.org/guide/solr/latest/deployment-guide/collection-management.html#create). (collection specific)
So, for #620 we could do the following:
- Add a new property `solr.status.routing.shards: 10` to solr-conf.yaml
- Add a script that creates the "status" collection with the numShards equal to the property above, and the `router.field` equal to the `solr.status.routing.fieldname: "key"`.
- The user should make sure that the SolrSpout parallelism equals the number of shards specified.
--
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: dev-unsubscribe@stormcrawler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org