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/06 15:50:15 UTC
[GitHub] [lucene-solr] dsmiley opened a new pull request #1723: SOLR prometheus: simplify concurrent collection
dsmiley opened a new pull request #1723:
URL: https://github.com/apache/lucene-solr/pull/1723
The intent of this is to simplify some concurrent code in the Prometheus exporter that I think is too confusing / contorted -- particularly Async.java. Git blame points at @shalinmangar so I would love a review to see what you think. I played with a few different approaches, and ultimately realized that we're working around using an Executor instead of an ExecutorService to benefit from invokeAll. I wish Java didn't have a distinction between Executor & ExecutorService but there is and we should all just ignore plain Executor IMO.
I haven't run this in-the-field but I could do so locally.
----------------------------------------------------------------
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] ErickErickson commented on pull request #1723: SOLR prometheus: simplify concurrent collection
Posted by GitBox <gi...@apache.org>.
ErickErickson commented on pull request #1723:
URL: https://github.com/apache/lucene-solr/pull/1723#issuecomment-670617417
If you only knew how many times I’ve beaten my head against a wall for _hours_, then read something that was in front of my face all the time more carefully… ;)
> On Aug 7, 2020, at 1:04 PM, David Smiley <no...@github.com> wrote:
>
>
> Woops; sorry; I should have looked more clearly! This is a "logok" situation I should do.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or unsubscribe.
>
----------------------------------------------------------------
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] dsmiley commented on pull request #1723: SOLR prometheus: simplify concurrent collection
Posted by GitBox <gi...@apache.org>.
dsmiley commented on pull request #1723:
URL: https://github.com/apache/lucene-solr/pull/1723#issuecomment-670598671
@ErickErickson the gradle validateLogCalls is complaining because I added a log statement without explicitly checking "log.isErrorEnabled" first. validate-log-calls.gradle is *not* supposed to do that for an error level log. Can you see what's going on please?
----------------------------------------------------------------
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] ErickErickson commented on pull request #1723: SOLR prometheus: simplify concurrent collection
Posted by GitBox <gi...@apache.org>.
ErickErickson commented on pull request #1723:
URL: https://github.com/apache/lucene-solr/pull/1723#issuecomment-670608605
The first part of the line gives the most explicit statement: "cause: 'getMessage or getCause in log line’”, the latter part of the line is more generic.
This check was added as part of SOLR-14523, since these two calls were often used in places that provided too little information. If you think this is an exception, you can add //logok to the line and it will pass.
Erick
> On Aug 7, 2020, at 12:23 PM, David Smiley <no...@github.com> wrote:
>
>
> @ErickErickson the gradle validateLogCalls is complaining because I added a log statement without explicitly checking "log.isErrorEnabled" first. validate-log-calls.gradle is not supposed to do that for an error level log. Can you see what's going on please?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or unsubscribe.
>
----------------------------------------------------------------
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] dsmiley merged pull request #1723: SOLR prometheus: simplify concurrent collection
Posted by GitBox <gi...@apache.org>.
dsmiley merged pull request #1723:
URL: https://github.com/apache/lucene-solr/pull/1723
----------------------------------------------------------------
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