You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/07/11 16:02:36 UTC
[GitHub] [solr] danrosher opened a new pull request, #935: SOLR-16286 : Topic stream not honoring initialCheckpoint in getPersis…
danrosher opened a new pull request, #935:
URL: https://github.com/apache/solr/pull/935
…tedCheckpoints
https://issues.apache.org/jira/browse/SOLR-XXXXX
<!--
_(If you are a project committer then you may remove some/all of the following template.)_
Before creating a pull request, please file an issue in the ASF Jira system for Solr:
* https://issues.apache.org/jira/projects/SOLR
For something minor (i.e. that wouldn't be worth putting in release notes), you can skip JIRA.
To create a Jira issue, you will need to create an account there first.
The title of the PR should reference the Jira issue number in the form:
* SOLR-####: <short description of problem or changes>
SOLR must be fully capitalized. A short description helps people scanning pull requests for items they can work on.
Properly referencing the issue in the title ensures that Jira is correctly updated with code review comments and commits. -->
# Description
Please provide a short description of the changes you're making with this pull request.
# Solution
Please provide a short description of the approach taken to implement your solution.
# Tests
Please describe the tests you've developed or run to confirm this patch implements the feature or solves the problem.
# Checklist
Please review the following and check all that apply:
- [ ] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
- [ ] I have created a Jira issue and added the issue ID to my pull request title.
- [ ] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
- [ ] I have developed this patch against the `main` branch.
- [ ] I have run `./gradlew check`.
- [ ] I have added tests for my changes.
- [ ] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] danrosher commented on a diff in pull request #935: SOLR-16286 : Topic stream not honoring initialCheckpoint in getPersis…
Posted by GitBox <gi...@apache.org>.
danrosher commented on code in PR #935:
URL: https://github.com/apache/solr/pull/935#discussion_r925846876
##########
solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamDecoratorTest.java:
##########
@@ -4372,7 +4372,7 @@ public void testClassifyStream() throws Exception {
classifyStream = new SolrStream(url, paramsLoc);
idToLabel = getIdToLabel(classifyStream, "probability_d");
- assertEquals(idToLabel.size(), 2);
Review Comment:
looks good to me
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] cpoerschke commented on pull request #935: SOLR-16286 : Topic stream not honoring initialCheckpoint in getPersis…
Posted by GitBox <gi...@apache.org>.
cpoerschke commented on PR #935:
URL: https://github.com/apache/solr/pull/935#issuecomment-1187531885
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] cpoerschke commented on pull request #935: SOLR-16286 : Topic stream not honoring initialCheckpoint in getPersis…
Posted by GitBox <gi...@apache.org>.
cpoerschke commented on PR #935:
URL: https://github.com/apache/solr/pull/935#issuecomment-1225845328
> ... 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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on pull request #935: SOLR-16286 : Topic stream not honoring initialCheckpoint in getPersis…
Posted by GitBox <gi...@apache.org>.
epugh commented on PR #935:
URL: https://github.com/apache/solr/pull/935#issuecomment-1297143336
@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 is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on pull request #935: SOLR-16286 : Topic stream not honoring initialCheckpoint in getPersis…
Posted by GitBox <gi...@apache.org>.
epugh commented on PR #935:
URL: https://github.com/apache/solr/pull/935#issuecomment-1217883013
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 is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] cpoerschke commented on a diff in pull request #935: SOLR-16286 : Topic stream not honoring initialCheckpoint in getPersis…
Posted by GitBox <gi...@apache.org>.
cpoerschke commented on code in PR #935:
URL: https://github.com/apache/solr/pull/935#discussion_r925835988
##########
solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamDecoratorTest.java:
##########
@@ -4372,7 +4372,7 @@ public void testClassifyStream() throws Exception {
classifyStream = new SolrStream(url, paramsLoc);
idToLabel = getIdToLabel(classifyStream, "probability_d");
- assertEquals(idToLabel.size(), 2);
Review Comment:
just added https://github.com/apache/solr/pull/935/commits/4c90a4af2454ca1c44519dcf46a25cbe4fb95ed6 with an alternative adjustment to this test. what do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] danrosher commented on pull request #935: SOLR-16286 : Topic stream not honoring initialCheckpoint in getPersis…
Posted by GitBox <gi...@apache.org>.
danrosher commented on PR #935:
URL: https://github.com/apache/solr/pull/935#issuecomment-1185622186
./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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] danrosher commented on a diff in pull request #935: SOLR-16286 : Topic stream not honoring initialCheckpoint in getPersis…
Posted by GitBox <gi...@apache.org>.
danrosher commented on code in PR #935:
URL: https://github.com/apache/solr/pull/935#discussion_r940144508
##########
solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamDecoratorTest.java:
##########
@@ -4370,6 +4370,17 @@ public void testClassifyStream() throws Exception {
updateRequest.add(id, String.valueOf(3), "text_s", "a b e e f");
updateRequest.commit(cluster.getSolrClient(), "uknownCollection");
+ expr =
+ "classify("
+ +
+ // use cacheMillis=0 to prevent cached results. it doesn't matter on the first run,
+ // but we want to ensure that when we re-use this expression later after
+ // training another model, we'll still get accurate results.
+ "model(modelCollection, id=\"model\", cacheMillis=0),"
+ + "topic(checkpointCollection, uknownCollection, q=\"*:*\", fl=\"text_s, id\", id=\"1000000\"),"
+ + "field=\"text_s\","
+ + "analyzerField=\"tv_text\")";
Review Comment:
checkpoints are persisted when the stream is closed, or if checkpointEvery > -1 (and then every count % checkpointEvery), otherwise the checkpoints are stored in the checkpoints hashmap, so for 'just' added docs, I think as long as is it matches the underlying query, and those docs have been soft committed (see caveat for topicstream SOLR-8709), I think they should be picked up, unless I'm completely misunderstanding ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on pull request #935: SOLR-16286 : Topic stream not honoring initialCheckpoint in getPersis…
Posted by GitBox <gi...@apache.org>.
epugh commented on PR #935:
URL: https://github.com/apache/solr/pull/935#issuecomment-1293948741
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 ;-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] cpoerschke commented on a diff in pull request #935: SOLR-16286 : Topic stream not honoring initialCheckpoint in getPersis…
Posted by GitBox <gi...@apache.org>.
cpoerschke commented on code in PR #935:
URL: https://github.com/apache/solr/pull/935#discussion_r925737991
##########
solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamDecoratorTest.java:
##########
@@ -3048,7 +3048,7 @@ public void testPriorityStream() throws Exception {
expression =
StreamExpressionParser.parse(
"priority(topic(collection1, collection1, q=\"a_s:hello\", fl=\"id,a_i\", id=1000000, initialCheckpoint=0),"
Review Comment:
#950 explores removing of the entire expression i.e. not just removing the `initialCheckpoint=0` (which would change the meaning to _"highest version in the index"_ actually) but removing the expression itself and continuing with the stream itself.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] cpoerschke commented on a diff in pull request #935: SOLR-16286 : Topic stream not honoring initialCheckpoint in getPersis…
Posted by GitBox <gi...@apache.org>.
cpoerschke commented on code in PR #935:
URL: https://github.com/apache/solr/pull/935#discussion_r925844560
##########
solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamDecoratorTest.java:
##########
@@ -4370,6 +4370,17 @@ public void testClassifyStream() throws Exception {
updateRequest.add(id, String.valueOf(3), "text_s", "a b e e f");
updateRequest.commit(cluster.getSolrClient(), "uknownCollection");
+ expr =
+ "classify("
+ +
+ // use cacheMillis=0 to prevent cached results. it doesn't matter on the first run,
+ // but we want to ensure that when we re-use this expression later after
+ // training another model, we'll still get accurate results.
+ "model(modelCollection, id=\"model\", cacheMillis=0),"
+ + "topic(checkpointCollection, uknownCollection, q=\"*:*\", fl=\"text_s, id\", id=\"1000000\"),"
+ + "field=\"text_s\","
+ + "analyzerField=\"tv_text\")";
Review Comment:
So this is the same expression as above but without the `initialCheckpoint=0` ... though reading the (current) docs that means _"the highest version in the index"_ though if the highest version in the index was used then the first batch in the stream below would not include the documents just added with ids 2 and 3?
Wondering if the documentation needs tweaking to account for persisted checkpoints?
https://github.com/apache/solr/blob/releases/solr/9.0.0/solr/solr-ref-guide/modules/query-guide/pages/stream-source-reference.adoc#topic-parameters
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] cpoerschke commented on a diff in pull request #935: SOLR-16286 : Topic stream not honoring initialCheckpoint in getPersis…
Posted by GitBox <gi...@apache.org>.
cpoerschke commented on code in PR #935:
URL: https://github.com/apache/solr/pull/935#discussion_r923396965
##########
solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamDecoratorTest.java:
##########
@@ -4372,7 +4372,7 @@ public void testClassifyStream() throws Exception {
classifyStream = new SolrStream(url, paramsLoc);
idToLabel = getIdToLabel(classifyStream, "probability_d");
- assertEquals(idToLabel.size(), 2);
Review Comment:
note to self: I don't understand yet why the `testClassifyStream` expectations change here.
##########
solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamExpressionTest.java:
##########
@@ -2528,6 +2528,97 @@ public void testSubFacetStream() throws Exception {
assertTrue(peri == 1.0D);
}
+ @Test
+ public void testTopicStreamInitialCheckpoint() throws Exception {
+ Assume.assumeTrue(!useAlias);
+
+ new UpdateRequest()
+ .add(id, "0", "a_s", "hello", "a_i", "0", "a_f", "1")
+ .add(id, "2", "a_s", "hello", "a_i", "2", "a_f", "2")
+ .add(id, "3", "a_s", "hello", "a_i", "3", "a_f", "3")
+ .add(id, "4", "a_s", "hello", "a_i", "4", "a_f", "4")
+ .add(id, "1", "a_s", "hello", "a_i", "1", "a_f", "5")
+ .add(id, "5", "a_s", "hello", "a_i", "10", "a_f", "6")
+ .add(id, "6", "a_s", "hello", "a_i", "11", "a_f", "7")
+ .add(id, "7", "a_s", "hello", "a_i", "12", "a_f", "8")
+ .add(id, "8", "a_s", "hello", "a_i", "13", "a_f", "9")
+ .add(id, "9", "a_s", "hello", "a_i", "14", "a_f", "10")
+ .commit(cluster.getSolrClient(), COLLECTIONORALIAS);
+
+ StreamFactory factory =
+ new StreamFactory()
+ .withCollectionZkHost("collection1", cluster.getZkServer().getZkAddress())
+ .withFunctionName("topic", TopicStream.class)
+ .withFunctionName("search", CloudSolrStream.class)
+ .withFunctionName("daemon", DaemonStream.class);
+
+ StreamExpression expression;
+ TupleStream stream = null;
+ List<Tuple> tuples;
+
+ SolrClientCache cache = new SolrClientCache();
+
+ try {
+
+ // Store checkpoints in the same index as the main documents. This perfectly valid
+ expression =
+ StreamExpressionParser.parse(
+ "topic(collection1, collection1, q=\"a_s:hello\", fl=\"id\", id=\"1000000\", initialCheckpoint=0)");
+
+ stream = new TopicStream(expression, factory);
+ StreamContext context = new StreamContext();
+ context.setSolrClientCache(cache);
+ stream.setStreamContext(context);
+ tuples = getTuples(stream);
+
+ assertEquals(10, tuples.size());
+
+ // force commit of checkpoints
+ cluster.getSolrClient().commit("collection1");
+
+ expression =
+ StreamExpressionParser.parse(
+ "search(collection1, q=\"id:1000000\", fl=\"id, checkpoint_ss, _version_\", sort=\"id asc\")");
+ stream = factory.constructStream(expression);
+ context = new StreamContext();
+ context.setSolrClientCache(cache);
+ stream.setStreamContext(context);
+ tuples = getTuples(stream);
+ assertEquals(tuples.size(), 1);
+ List<String> checkpoints = tuples.get(0).getStrings("checkpoint_ss");
+ assertEquals(checkpoints.size(), 2);
Review Comment:
```suggestion
assertEquals(checkpoints.size(), 2); // one checkpoint for each shard
```
##########
solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamDecoratorTest.java:
##########
@@ -3048,7 +3048,7 @@ public void testPriorityStream() throws Exception {
expression =
StreamExpressionParser.parse(
"priority(topic(collection1, collection1, q=\"a_s:hello\", fl=\"id,a_i\", id=1000000, initialCheckpoint=0),"
Review Comment:
Looking at what the `testPriorityStream` test is testing, I wonder if the `initialCheckpoint=0` here and at lines 3035/3036 need also be removed?
```suggestion
"priority(topic(collection1, collection1, q=\"a_s:hello\", fl=\"id,a_i\", id=1000000),"
```
Having said that, it's not very obvious when the checkpoints from the line 3019/3020 stream will be persisted and/or otherwise carry-over to the line 3035/3036 stream.
##########
solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamExpressionTest.java:
##########
@@ -2528,6 +2528,97 @@ public void testSubFacetStream() throws Exception {
assertTrue(peri == 1.0D);
}
+ @Test
+ public void testTopicStreamInitialCheckpoint() throws Exception {
+ Assume.assumeTrue(!useAlias);
+
+ new UpdateRequest()
+ .add(id, "0", "a_s", "hello", "a_i", "0", "a_f", "1")
+ .add(id, "2", "a_s", "hello", "a_i", "2", "a_f", "2")
+ .add(id, "3", "a_s", "hello", "a_i", "3", "a_f", "3")
+ .add(id, "4", "a_s", "hello", "a_i", "4", "a_f", "4")
+ .add(id, "1", "a_s", "hello", "a_i", "1", "a_f", "5")
+ .add(id, "5", "a_s", "hello", "a_i", "10", "a_f", "6")
+ .add(id, "6", "a_s", "hello", "a_i", "11", "a_f", "7")
+ .add(id, "7", "a_s", "hello", "a_i", "12", "a_f", "8")
+ .add(id, "8", "a_s", "hello", "a_i", "13", "a_f", "9")
+ .add(id, "9", "a_s", "hello", "a_i", "14", "a_f", "10")
+ .commit(cluster.getSolrClient(), COLLECTIONORALIAS);
+
+ StreamFactory factory =
+ new StreamFactory()
+ .withCollectionZkHost("collection1", cluster.getZkServer().getZkAddress())
+ .withFunctionName("topic", TopicStream.class)
+ .withFunctionName("search", CloudSolrStream.class)
+ .withFunctionName("daemon", DaemonStream.class);
+
+ StreamExpression expression;
+ TupleStream stream = null;
+ List<Tuple> tuples;
+
+ SolrClientCache cache = new SolrClientCache();
+
+ try {
+
+ // Store checkpoints in the same index as the main documents. This perfectly valid
+ expression =
+ StreamExpressionParser.parse(
+ "topic(collection1, collection1, q=\"a_s:hello\", fl=\"id\", id=\"1000000\", initialCheckpoint=0)");
+
+ stream = new TopicStream(expression, factory);
+ StreamContext context = new StreamContext();
+ context.setSolrClientCache(cache);
+ stream.setStreamContext(context);
+ tuples = getTuples(stream);
+
+ assertEquals(10, tuples.size());
+
+ // force commit of checkpoints
+ cluster.getSolrClient().commit("collection1");
+
+ expression =
+ StreamExpressionParser.parse(
+ "search(collection1, q=\"id:1000000\", fl=\"id, checkpoint_ss, _version_\", sort=\"id asc\")");
+ stream = factory.constructStream(expression);
+ context = new StreamContext();
+ context.setSolrClientCache(cache);
+ stream.setStreamContext(context);
+ tuples = getTuples(stream);
+ assertEquals(tuples.size(), 1);
+ List<String> checkpoints = tuples.get(0).getStrings("checkpoint_ss");
+ assertEquals(checkpoints.size(), 2);
+
+ expression =
+ StreamExpressionParser.parse(
+ "topic(collection1, collection1, q=\"a_s:hello\", fl=\"id\", id=\"1000000\", initialCheckpoint=0)");
+
+ stream = new TopicStream(expression, factory);
+ context = new StreamContext();
+ context.setSolrClientCache(cache);
+ stream.setStreamContext(context);
+ tuples = getTuples(stream);
+
+ assertEquals(10, tuples.size());
+
+ expression =
+ StreamExpressionParser.parse(
+ "topic(collection1, collection1, q=\"a_s:hello\", fl=\"id\", id=\"1000000\")");
+
+ stream = new TopicStream(expression, factory);
+ context = new StreamContext();
+ context.setSolrClientCache(cache);
+ stream.setStreamContext(context);
+ tuples = getTuples(stream);
+
+ // Should be zero because the checkpoints will be set to the highest vesion on the shards.
Review Comment:
```suggestion
// Should be zero because the checkpoints will be set to the highest version on the shards.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] danrosher commented on a diff in pull request #935: SOLR-16286 : Topic stream not honoring initialCheckpoint in getPersis…
Posted by GitBox <gi...@apache.org>.
danrosher commented on code in PR #935:
URL: https://github.com/apache/solr/pull/935#discussion_r925427240
##########
solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamDecoratorTest.java:
##########
@@ -3048,7 +3048,7 @@ public void testPriorityStream() throws Exception {
expression =
StreamExpressionParser.parse(
"priority(topic(collection1, collection1, q=\"a_s:hello\", fl=\"id,a_i\", id=1000000, initialCheckpoint=0),"
Review Comment:
I wasn't sure whether to remove initialCheckpoint=0, or alter the test expected results which is what I did. Shall I remove the initialCheckpoint=0?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] danrosher commented on pull request #935: SOLR-16286 : Topic stream not honoring initialCheckpoint in getPersis…
Posted by GitBox <gi...@apache.org>.
danrosher commented on PR #935:
URL: https://github.com/apache/solr/pull/935#issuecomment-1218098214
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:
> ***@***.***>
>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-16286 : Topic stream not honoring initialCheckpoint in getPersis… [solr]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #935:
URL: https://github.com/apache/solr/pull/935#issuecomment-1951491412
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 dev@solr.apache.org mailing list. Thank you for your contribution!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org