solr
solr copied to clipboard
SOLR-16286 : Topic stream not honoring initialCheckpoint in getPersis…
https://issues.apache.org/jira/browse/SOLR-16286
Description
getCheckpoints() does honor initialCheckpoint, but when stored, getPersistedCheckpoints which is processed first, does not. The effect is that initialCheckpoint=0 doesn't work as expected after it's stored.
Solution
Modify getPersistedCheckpoints to honor initialCheckpoint as it does in getCheckpoints
Tests
Added StreamExpressionTest.testTopicStreamInitialCheckpoint to do run a topic SE, then persist it, then re-run to ensure initialCheckpoint=0 is still honored, then again without initialCheckpoint=0 to ensure zero returned as up to date.,
Checklist
Please review the following and check all that apply:
- [x] I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
- [x] I have created a Jira issue and added the issue ID to my pull request title.
- [x] I have given Solr maintainers access to contribute to my PR branch. (optional but recommended)
- [x] I have developed this patch against the
mainbranch. - [x] I have run
./gradlew check. - [x] I have added tests for my changes.
- [ ] I have added documentation for the Reference Guide
./gradlew check passed except:
` Crawl/parse...
Verify...
.../solr/solr/documentation/build/site/modules/jwt-auth/org/apache/solr/handler/admin/api/ModifyJWTAuthPluginConfigAPI.html BROKEN LINK: .../solr/solr/documentation/build/site/core/org/apache/solr/handler/admin/api/JWTConfigurationPayload.html BROKEN LINK: .../solr/solr/documentation/build/site/core/org/apache/solr/handler/admin/api/JWTConfigurationPayload.html
Broken javadocs links were found! Common root causes:
- A typo of some sort for manually created links.
- Public methods referencing non-public classes in their signature.
~
`
unsure why this failed though
Broken javadocs links were found! Common root causes:
- A typo of some sort for manually created links.
- Public methods referencing non-public classes in their signature. ~ ` unsure why this failed though
#944 sounds like it might be related.
Thoughts @cpoerschke and @danrosher on this being ready for merging? Seems like a valuable bug fix! Or is there still a lack of clarity on the right behavior?
This PR passes the tests in StreamDecoratorTest and StreamExpressionTest so I believe it's ready to merge
On Wed, 17 Aug 2022 at 12:26, Eric Pugh @.***> wrote:
Thoughts @cpoerschke https://github.com/cpoerschke and @danrosher https://github.com/danrosher on this being ready for merging? Seems like a valuable bug fix! Or is there still a lack of clarity on the right behavior?
— Reply to this email directly, view it on GitHub https://github.com/apache/solr/pull/935#issuecomment-1217883013, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACE6HP37J3K7TBZQAZ5EZ3LVZTD5FANCNFSM53IAXYQA . You are receiving this because you were mentioned.Message ID: @.***>
... Or is there still a lack of clarity on the right behavior?
I think the new behaviour makes sense.
Am less sure w.r.t. the documentation and/or how to describe the changes in behaviour. @epugh - feel free to jump in if you wish.
https://github.com/apache/solr/blob/5a5989e5b6164091243dd29cfe327b5eaac2cfbd/solr/solr-ref-guide/modules/query-guide/pages/stream-source-reference.adoc#topic-parameters currently says "If not set, it defaults to the highest version in the index." -- is that the highest version of documents in the index or is it alluding to checkpoints somehow? Would something like "If not set, it defaults to previously established checkpoints (if any) or otherwise the highest version in the index." be accurate and user-friendly?
i think some question about the docs. I like everything here ;-). I am going to add it to my "List of tickets to merge on Monday Oct 31st" so folks can weigh in ;-)
@joel-bernstein I was going to merge this today, but I realized that might conflict with your work on seperating out the streaming code? Should I wait? Do you want to add this to your list of tickets to merge once that work is done?
This PR had no visible activity in the past 60 days, labeling it as stale. Any new activity will remove the stale label. To attract more reviewers, please tag someone or notify the [email protected] mailing list. Thank you for your contribution!