Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/977#issuecomment-169146734
I am not an expert on the Solr code, and Solr itself. So below comment may
need to have corrections via other contributors / committers.
Looking into SolrCommitStrategy and CountBasedCommit I feel the same thing
with @revans2.
State for the SolrCommitStrategy could be out of sync because it cannot
know `internal commit` (not requested via commit()) yet, and looking into
javadoc for SolrClient, there's no way to check internal commit.
Here's description for rollback, which describes the internal commit.
https://lucene.apache.org/solr/5_2_0/solr-solrj/org/apache/solr/client/solrj/SolrClient.html#rollback()
> rollback
>
>Performs a rollback of all non-committed documents pending. Note that this
is not a true rollback as in databases. Content you have previously added may
have been committed due to autoCommit, buffer full, other client performing a
commit etc.
Based on internal commit, I see current code has an issue, which could be
minor, or maybe major.
> There could be some requests which are committed internally via Solr but
not acked / failed via Bolt's side. (It will be resolved when SolrUpdateBolt
deals with tick tuple.) It means acknowledges of some tuples can be lost though
tuples are committed to Solr.
If we intend to let requests buffered and in perfect sync with ack / fail,
I think it shouldn't request for each execute() but just stores requests and do
actual requests / external commit to Solr / ack & fail when commit condition
has been made.
Or if we don't need to be accurate, I'm fine with the code change.
---
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.
---
