Github user dossett commented on a diff in the pull request:
https://github.com/apache/storm/pull/977#discussion_r48859729
--- Diff:
external/storm-solr/src/main/java/org/apache/storm/solr/bolt/SolrUpdateBolt.java
---
@@ -92,11 +94,19 @@ private void ack(Tuple tuple) throws
SolrServerException, IOException {
if (commitStgy == null) {
collector.ack(tuple);
} else {
- toCommitTuples.add(tuple);
- commitStgy.update();
- if (commitStgy.commit()) {
+ if (TupleUtils.isTick(tuple)) {
+ LOG.debug("TICK! forcing solr client commit");
+ collector.ack(tuple);
+ commitStgy.commit();
solrClient.commit(solrMapper.getCollection());
ackCommittedTuples();
+ } else {
+ toCommitTuples.add(tuple);
+ commitStgy.update();
+ if (commitStgy.commit()) {
--- End diff --
The strategy in AbstractHdfsBolt is to set a boolean in the case of a tick
tuple and then sync if that value is true or if other conditions dictate a
sync.
(https://github.com/apache/storm/blob/master/external/storm-hdfs/src/main/java/org/apache/storm/hdfs/bolt/AbstractHdfsBolt.java#L154)
The benefit of that approach is to eliminate duplicate code (i.e. that
calls to ackCommittedTuples() and solrClient.commit()), which I think is a
substantial benefit.
Here that would looks something like:
```code
if (forceCommit || commitStgy.commit()) {
solrClient.commit(solrMapper.getCollection());
ackCommittedTuples();
}
```
With duplicate code removed I would be +1
A unit test would also be helpful. HdfsBolt example is here:
https://github.com/apache/storm/blob/master/external/storm-hdfs/src/test/java/org/apache/storm/hdfs/bolt/TestHdfsBolt.java#L175
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
