[
https://issues.apache.org/jira/browse/SOLR-11654?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16515888#comment-16515888
]
David Smiley commented on SOLR-11654:
-------------------------------------
Thanks Gus! This is looking great; I really like your
TrackingUpdateProcessorFactory in particular. Very useful. I have some local
super minor tweaks but I want to mention a few things:
* removed SolrTestCaseJ4.adocs which you added but wasn't used. Besides, I
don't like these methods on SolrTestCaseJ4 that return XML strings.
* removed SolrClient.add(collection,SolrInputDocument...) because I don't think
an API should have both array/varargs form and Collection form (and besides,
Arrays.asList() is not that verbose).
* I did leave just one method you added on SolrClient...
{code:java}
/**
* Adds a collection of documents, specifying max time before they become
committed and additional
* parameters for the update.
*
* @param collection the Solr collection to add documents to
* @param docs the collection of documents
* @param commitWithinMs max time (in ms) before a commit will happen
* @param params any parameters to be sent with this update request.
*
* @return an {@link org.apache.solr.client.solrj.response.UpdateResponse}
from the server
*
* @throws IOException if there is a communication error with the
server
* @throws SolrServerException if there is an error on the server
*
* @since Solr 5.1
*/
public UpdateResponse add(String collection, Collection<SolrInputDocument>
docs, int commitWithinMs, ModifiableSolrParams params) throws
SolrServerException, IOException {
UpdateRequest req = new UpdateRequest();
if (params != null) {
req.setParams(params);
}
req.add(docs);
req.setCommitWithin(commitWithinMs);
return req.process(this, collection);
}
{code}
Since I think peer review is especially important on the public API of
SolrClient, I'd like input from [~gerlowskija] or [~varunthacker] that the
above new method is okay. What the above method saves is manual creation of an
UpdateRequest as used above. I know it's not often to use params on an update.
There is one thing I think that should be changed from Gus's proposal -- I
think setParams should copy the input to a new ModifiableSolrParams, and I
think the declared param type of the method here should simply be SolrParams.
I think it's trappy that UpdateRequest takes a mutable SolrParams and then
modifies it.
> TimePartitionedUpdateProcessor.lookupShardLeaderOfCollection should route to
> the ideal shard
> --------------------------------------------------------------------------------------------
>
> Key: SOLR-11654
> URL: https://issues.apache.org/jira/browse/SOLR-11654
> Project: Solr
> Issue Type: Sub-task
> Security Level: Public(Default Security Level. Issues are Public)
> Components: SolrCloud
> Reporter: David Smiley
> Assignee: David Smiley
> Priority: Major
> Attachments: SOLR-11654.patch, SOLR-11654.patch, SOLR-11654.patch
>
>
> {{TimePartitionedUpdateProcessor.lookupShardLeaderOfCollection}} looks up the
> Shard/Slice to talk to for the given collection. It currently picks the
> first active Shard/Slice but it has a TODO to route to the ideal one based on
> the router configuration of the target collection. There is similar code in
> CloudSolrClient & DistributedUpdateProcessor that should probably be
> refactored/standardized so that we don't have to repeat this logic.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
