You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2020/08/14 22:37:43 UTC
[GitHub] [lucene-solr] MarcusSorealheis opened a new pull request #1755: SOLR-14753: Improve thread annotation name
MarcusSorealheis opened a new pull request #1755:
URL: https://github.com/apache/lucene-solr/pull/1755
<!--
_(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 Lucene or Solr:
* https://issues.apache.org/jira/projects/LUCENE
* https://issues.apache.org/jira/projects/SOLR
You will need to create an account in Jira in order to create an issue.
The title of the PR should reference the Jira issue number in the form:
* LUCENE-####: <short description of problem or changes>
* SOLR-####: <short description of problem or changes>
LUCENE and 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
The thread safety annotation names are misleading/confusing for readers, yet the purpose is to clarify things for readers. Here is a fix proposed by @cpoerschke
# Solution
Change `SolrSingleThreaded` to `SolrThreadUnsafe`
# Tests
No tests required.
# 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 `master` branch.
- [ ] I have run `ant precommit` and the appropriate test suite.
- [ ] I have added tests for my changes.
- [ ] I have added documentation for the [Ref Guide](https://github.com/apache/lucene-solr/tree/master/solr/solr-ref-guide) (for Solr changes only).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] anshumg commented on a change in pull request #1755: SOLR-14753: Improve thread annotation name
Posted by GitBox <gi...@apache.org>.
anshumg commented on a change in pull request #1755:
URL: https://github.com/apache/lucene-solr/pull/1755#discussion_r473283554
##########
File path: solr/solrj/src/java/org/apache/solr/common/annotation/SolrThreadUnsafe.java
##########
@@ -29,6 +29,6 @@
@Documented
@Retention(SOURCE)
@Target(TYPE)
-public @interface SolrSingleThreaded {
Review comment:
Was late to the party here :)
Thanks for taking care of this.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] MarcusSorealheis commented on a change in pull request #1755: SOLR-14753: Improve thread annotation name
Posted by GitBox <gi...@apache.org>.
MarcusSorealheis commented on a change in pull request #1755:
URL: https://github.com/apache/lucene-solr/pull/1755#discussion_r470914277
##########
File path: solr/solrj/src/java/org/apache/solr/common/annotation/SolrSingleThreaded.java
##########
@@ -29,6 +29,6 @@
@Documented
@Retention(SOURCE)
@Target(TYPE)
-public @interface SolrSingleThreaded {
+public @interface SolrThreadUnsafe {
Review comment:
stepped away to get dinner. back now
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] anshumg commented on a change in pull request #1755: SOLR-14753: Improve thread annotation name
Posted by GitBox <gi...@apache.org>.
anshumg commented on a change in pull request #1755:
URL: https://github.com/apache/lucene-solr/pull/1755#discussion_r470907966
##########
File path: solr/solrj/src/java/org/apache/solr/common/annotation/SolrSingleThreaded.java
##########
@@ -29,6 +29,6 @@
@Documented
@Retention(SOURCE)
@Target(TYPE)
-public @interface SolrSingleThreaded {
+public @interface SolrThreadUnsafe {
Review comment:
Please change the name of the file too.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] MarcusSorealheis commented on a change in pull request #1755: SOLR-14753: Improve thread annotation name
Posted by GitBox <gi...@apache.org>.
MarcusSorealheis commented on a change in pull request #1755:
URL: https://github.com/apache/lucene-solr/pull/1755#discussion_r470914210
##########
File path: solr/solrj/src/java/org/apache/solr/common/annotation/SolrSingleThreaded.java
##########
@@ -29,6 +29,6 @@
@Documented
@Retention(SOURCE)
@Target(TYPE)
-public @interface SolrSingleThreaded {
+public @interface SolrThreadUnsafe {
Review comment:
oops
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] MarcusSorealheis commented on a change in pull request #1755: SOLR-14753: Improve thread annotation name
Posted by GitBox <gi...@apache.org>.
MarcusSorealheis commented on a change in pull request #1755:
URL: https://github.com/apache/lucene-solr/pull/1755#discussion_r471908464
##########
File path: solr/solrj/src/java/org/apache/solr/common/annotation/SolrThreadUnsafe.java
##########
@@ -29,6 +29,6 @@
@Documented
@Retention(SOURCE)
@Target(TYPE)
-public @interface SolrSingleThreaded {
Review comment:
I think it's a good thought. The reason I think it is so prescient is the fact that this annotation originated in a private branch where I expect many pieces will land in open source. The original annotation name may be used in that code. Keeping this name for now will make it easier to bring that work into open source.
If it's in their custom code, I would hope the name change would be trivial.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] MarcusSorealheis commented on a change in pull request #1755: SOLR-14753: Improve thread annotation name
Posted by GitBox <gi...@apache.org>.
MarcusSorealheis commented on a change in pull request #1755:
URL: https://github.com/apache/lucene-solr/pull/1755#discussion_r470914648
##########
File path: solr/solrj/src/java/org/apache/solr/common/annotation/SolrSingleThreaded.java
##########
@@ -29,6 +29,6 @@
@Documented
@Retention(SOURCE)
@Target(TYPE)
-public @interface SolrSingleThreaded {
+public @interface SolrThreadUnsafe {
Review comment:
@anshumg Should be good now. Mid-work I was starting to get headache from now eating.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] chatman commented on pull request #1755: SOLR-14753: Improve thread annotation name
Posted by GitBox <gi...@apache.org>.
chatman commented on pull request #1755:
URL: https://github.com/apache/lucene-solr/pull/1755#issuecomment-675411238
Thanks for the PR, Marcus. I committed this change as part of SOLR-14731. Can you please close both the PRs (as usual, I forgot to do it via commit message and my github credentials are still not in order, sorry)?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] anshumg commented on a change in pull request #1755: SOLR-14753: Improve thread annotation name
Posted by GitBox <gi...@apache.org>.
anshumg commented on a change in pull request #1755:
URL: https://github.com/apache/lucene-solr/pull/1755#discussion_r473282984
##########
File path: solr/solrj/src/java/org/apache/solr/common/annotation/SolrThreadUnsafe.java
##########
@@ -29,6 +29,6 @@
@Documented
@Retention(SOURCE)
@Target(TYPE)
-public @interface SolrSingleThreaded {
Review comment:
I thought I left a comment here but seems like I missed it.
I agree that it will make it easier for people to use the wrong annotation, but that's something that the person who writes/reviews/commits should notice. It's also something we could change, but removing this annotation is going to lead to a breaking change which we might have to just live with for back-compat reasons.
The private branch is no longer really in use, so we don't have to worry about anything from that specific branch making it into upstream.
Let's leave the old annotation as deprecated.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] MarcusSorealheis commented on a change in pull request #1755: SOLR-14753: Improve thread annotation name
Posted by GitBox <gi...@apache.org>.
MarcusSorealheis commented on a change in pull request #1755:
URL: https://github.com/apache/lucene-solr/pull/1755#discussion_r470914648
##########
File path: solr/solrj/src/java/org/apache/solr/common/annotation/SolrSingleThreaded.java
##########
@@ -29,6 +29,6 @@
@Documented
@Retention(SOURCE)
@Target(TYPE)
-public @interface SolrSingleThreaded {
+public @interface SolrThreadUnsafe {
Review comment:
@anshumg Should be good now. Mid-work I was starting to get headache from not eating.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] MarcusSorealheis closed pull request #1755: SOLR-14753: Improve thread annotation name
Posted by GitBox <gi...@apache.org>.
MarcusSorealheis closed pull request #1755:
URL: https://github.com/apache/lucene-solr/pull/1755
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] cpoerschke commented on a change in pull request #1755: SOLR-14753: Improve thread annotation name
Posted by GitBox <gi...@apache.org>.
cpoerschke commented on a change in pull request #1755:
URL: https://github.com/apache/lucene-solr/pull/1755#discussion_r471379456
##########
File path: solr/solrj/src/java/org/apache/solr/common/annotation/SolrThreadUnsafe.java
##########
@@ -29,6 +29,6 @@
@Documented
@Retention(SOURCE)
@Target(TYPE)
-public @interface SolrSingleThreaded {
Review comment:
Via https://issues.apache.org/jira/browse/SOLR-13998 the `SolrSingleThreaded` public interface was included in releases from https://github.com/apache/lucene-solr/blob/releases/lucene-solr/8.4.0/solr/solrj/src/java/org/apache/solr/common/annotation/SolrSingleThreaded.java onwards. And so potentially folks could be using it in their custom code built against 8.4 onwards, though this might be unlikely in practice perhaps, it's difficult to know.
I wonder if we have or want to have some sort of backwards compatibility support for annotations? E.g. the scope of this pull request here could become _"Add SolrThreadUnsafe annotation, deprecate SolrSingleThreaded annotation."_ i.e. we keep `SolrSingleThreaded` around as deprecated for the remainder of the 8x releases and a separate commit for master branch only would remove the deprecated interface from 9.0 onwards.
Thoughts?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org