[ 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: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org