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/08/01 20:03:43 UTC
[GitHub] [solr-sandbox] markrmiller opened a new pull request, #32: Reindex Test, cover a few more test cases, additional config options.
markrmiller opened a new pull request, #32:
URL: https://github.com/apache/solr-sandbox/pull/32
Adds a basic test focused on reindexing, covers a few more test cases: more replicas and shards, different shard count on primary and secondary dc, allows consumer group to be configured, allows crossdc zk prop file location to be configured.
--
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-sandbox] anshumg commented on a diff in pull request #32: Reindex Test, cover a few more test cases, additional config options.
Posted by GitBox <gi...@apache.org>.
anshumg commented on code in PR #32:
URL: https://github.com/apache/solr-sandbox/pull/32#discussion_r935944295
##########
crossdc-consumer/src/main/java/org/apache/solr/crossdc/consumer/KafkaCrossDcConsumer.java:
##########
@@ -49,7 +49,7 @@ public KafkaCrossDcConsumer(KafkaCrossDcConf conf) {
kafkaConsumerProp.put(ConsumerConfig.BOOTSTRAP_SERVERS_CONFIG, conf.getBootStrapServers());
- kafkaConsumerProp.put(ConsumerConfig.GROUP_ID_CONFIG, "group_1"); // TODO
+ kafkaConsumerProp.put(ConsumerConfig.GROUP_ID_CONFIG, conf.getGroupId()); // TODO
Review Comment:
We can get rid of the `//TODO` here?
--
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-sandbox] markrmiller commented on pull request #32: Reindex Test, cover a few more test cases, additional config options.
Posted by GitBox <gi...@apache.org>.
markrmiller commented on PR #32:
URL: https://github.com/apache/solr-sandbox/pull/32#issuecomment-1205867090
> The counter is that it's required to pass a group id so you need some default.
Though, an empty string is a valid default to use as far as I know.
--
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-sandbox] markrmiller commented on pull request #32: Reindex Test, cover a few more test cases, additional config options.
Posted by GitBox <gi...@apache.org>.
markrmiller commented on PR #32:
URL: https://github.com/apache/solr-sandbox/pull/32#issuecomment-1205871709
Okay, I was reading that as kind of two different comments, and it just jelled for me. You are saying you think we should make groupid required in our config?
I guess I would say that feels a little unnecessary given it doesn't necessarily always have value for a setup and if it does a user would know that they want that value and be able to use it, and someone trying things out or that didn't need it wouldn't have to add one more bit of config that they may or may not understand initially. To pick a reasonable group id, you really have to understand something about Kafka and what it is and if you understand that you know whether you want a group id or not and if you don't know all that, it's just a seemingly not very relevant addition to the current very simple required config for someone getting started.
--
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-sandbox] markrmiller commented on pull request #32: Reindex Test, cover a few more test cases, additional config options.
Posted by GitBox <gi...@apache.org>.
markrmiller commented on PR #32:
URL: https://github.com/apache/solr-sandbox/pull/32#issuecomment-1205888191
For the moment I just put up an update I already had together. As far as requiring group id, I guess I see it the opposite way, eg what's a good reason to require it? Mainly from the stand point that minimal required configuration to get going is easier to get started with and so my intent with making so little configurable to start was to try and keep what was required minimal and that what was possible on that either kind of minimally necessary and/or offer a way to generically take and pass on anything Kafka can accept in a way that that we don't own each option as well, just what we make required.
--
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-sandbox] anshumg merged pull request #32: Reindex Test, cover a few more test cases, additional config options.
Posted by GitBox <gi...@apache.org>.
anshumg merged PR #32:
URL: https://github.com/apache/solr-sandbox/pull/32
--
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-sandbox] markrmiller commented on pull request #32: Reindex Test, cover a few more test cases, additional config options.
Posted by GitBox <gi...@apache.org>.
markrmiller commented on PR #32:
URL: https://github.com/apache/solr-sandbox/pull/32#issuecomment-1204842767
Thanks for the review. Meant to get my update up today, but got sidetracked on another item. Will get up tomorrow.
--
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-sandbox] markrmiller commented on pull request #32: Reindex Test, cover a few more test cases, additional config options.
Posted by GitBox <gi...@apache.org>.
markrmiller commented on PR #32:
URL: https://github.com/apache/solr-sandbox/pull/32#issuecomment-1205865580
> We need to handle empty/malformed/missing groupId, topicName, etc. here.
Do we? That means copying and duplicating and evolving the rules from/with Kafka when Kafka already handles it for us. We pass the input to Kafka, it validates it, it spits out an error message.
> I personally feel it doesn't make sense to have a default but I'm open to a counter argument.
The counter is that it's required to pass a group id so you need some default.
--
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