[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15101848#comment-15101848 ] ASF subversion and git services commented on SOLR-8330: --- Commit 1724826 from [~yo...@apache.org] in branch 'dev/trunk' [ https://svn.apache.org/r1724826 ] SOLR-8330: fix log4j.properties to match new name of log update processor logger > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: 5.4, Trunk > > Attachments: SOLR-8330-combined.patch, SOLR-8330-detector.patch, > SOLR-8330-detector.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15101850#comment-15101850 ] ASF subversion and git services commented on SOLR-8330: --- Commit 1724827 from [~yo...@apache.org] in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1724827 ] SOLR-8330: fix log4j.properties to match new name of log update processor logger > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: 5.4, Trunk > > Attachments: SOLR-8330-combined.patch, SOLR-8330-detector.patch, > SOLR-8330-detector.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15049736#comment-15049736 ] Mike Drob commented on SOLR-8330: - This issue looks done - can we resolve it if there is no further work to do? > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: 5.4, Trunk > > Attachments: SOLR-8330-combined.patch, SOLR-8330-detector.patch, > SOLR-8330-detector.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15037606#comment-15037606 ] Ishan Chattopadhyaya commented on SOLR-8330: The MethodHandles.lookup() creates an extra object for the Lookup inner class of MethodHandles. Isn't it memory/gc wise expensive, esp. given that some of the shortlived, often created objects (like AtomicUpdateDocumentMerger, DistributedUpdateProcessor etc.), would be creating their own logger, thus implying many extra MethodHandles.Lookup objects for every request? Is that a significant problem, or am I over estimating the problem? > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: 5.4, Trunk > > Attachments: SOLR-8330-combined.patch, SOLR-8330-detector.patch, > SOLR-8330-detector.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15037614#comment-15037614 ] Uwe Schindler commented on SOLR-8330: - The loggers are static, so created once. > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: 5.4, Trunk > > Attachments: SOLR-8330-combined.patch, SOLR-8330-detector.patch, > SOLR-8330-detector.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15037625#comment-15037625 ] Ishan Chattopadhyaya commented on SOLR-8330: Ah, I was missing the static part; indeed no per-request cost. Good to learn about the escape analysis; thanks! > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: 5.4, Trunk > > Attachments: SOLR-8330-combined.patch, SOLR-8330-detector.patch, > SOLR-8330-detector.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[
https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15037649#comment-15037649
]
Uwe Schindler commented on SOLR-8330:
-
bq. Good to learn about the escape analysis; thanks!
This is one of the most important Hotspot optimizations, especially for Java 8!
Lambdas and the fluent stream API based on collections would be slow without
escape analysis. Fluent APIs like {{int evenSquareSum =
Arrays.stream(intArray).filter(i -> i % 2 == 0).map(i -> i * i).sum();}} make
profit from that because those APIs return new instances of Stream subclasses
all the time. Same applies for java.time API. Those never escape the method.
> Restrict logger visibility throughout the codebase to private so that only
> the file that declares it can use it
> ---
>
> Key: SOLR-8330
> URL: https://issues.apache.org/jira/browse/SOLR-8330
> Project: Solr
> Issue Type: Sub-task
>Affects Versions: Trunk
>Reporter: Jason Gerlowski
>Assignee: Anshum Gupta
> Labels: logging
> Fix For: 5.4, Trunk
>
> Attachments: SOLR-8330-combined.patch, SOLR-8330-detector.patch,
> SOLR-8330-detector.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch,
> SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch
>
>
> As Mike Drob pointed out in Solr-8324, many loggers in Solr are
> unintentionally shared between classes. Many instances of this are caused by
> overzealous copy-paste. This can make debugging tougher, as messages appear
> to come from an incorrect location.
> As discussed in the comments on SOLR-8324, there also might be legitimate
> reasons for sharing loggers between classes. Where any ambiguity exists,
> these instances shouldn't be touched.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15035526#comment-15035526 ] Uwe Schindler commented on SOLR-8330: - +1 Thanks! > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: Trunk > > Attachments: SOLR-8330-combined.patch, SOLR-8330-detector.patch, > SOLR-8330-detector.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15035524#comment-15035524 ] ASF subversion and git services commented on SOLR-8330: --- Commit 1717590 from [~anshumg] in branch 'dev/trunk' [ https://svn.apache.org/r1717590 ] SOLR-8330: Standardize and fix logger creation and usage so that they aren't shared across source files. > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: Trunk > > Attachments: SOLR-8330-combined.patch, SOLR-8330-detector.patch, > SOLR-8330-detector.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15037218#comment-15037218 ] ASF subversion and git services commented on SOLR-8330: --- Commit 1717707 from [~anshumg] in branch 'dev/branches/lucene_solr_5_4' [ https://svn.apache.org/r1717707 ] SOLR-8330: Standardize and fix logger creation and usage so that they aren't shared across source files.(merge from branch_5x) > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: Trunk > > Attachments: SOLR-8330-combined.patch, SOLR-8330-detector.patch, > SOLR-8330-detector.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15037236#comment-15037236 ] Anshum Gupta commented on SOLR-8330: It's now in 5.4. > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: 5.4, Trunk > > Attachments: SOLR-8330-combined.patch, SOLR-8330-detector.patch, > SOLR-8330-detector.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15035630#comment-15035630 ] ASF subversion and git services commented on SOLR-8330: --- Commit 1717604 from [~anshumg] in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1717604 ] SOLR-8330: Standardize and fix logger creation and usage so that they aren't shared across source files.(merge from trunk) > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: Trunk > > Attachments: SOLR-8330-combined.patch, SOLR-8330-detector.patch, > SOLR-8330-detector.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15035654#comment-15035654 ] Anshum Gupta commented on SOLR-8330: I've committed it to both trunk and 5x. I think we should also commit this to 5.4, any thoughts? Thank you Jason, Uwe and everyone else ! :) > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: Trunk > > Attachments: SOLR-8330-combined.patch, SOLR-8330-detector.patch, > SOLR-8330-detector.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15035726#comment-15035726 ] Jason Gerlowski commented on SOLR-8330: --- Thank you Anshum! I think it'd be nice to squeeze into 5.4. Even though this is a big change from a LOC perspective, I think it's pretty safe, and is a net gain for log-readability. It'd be nice to get out to people. Though, as a partial author, I must admit a little bias :-p > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: Trunk > > Attachments: SOLR-8330-combined.patch, SOLR-8330-detector.patch, > SOLR-8330-detector.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15033319#comment-15033319 ] Uwe Schindler commented on SOLR-8330: - The attached patch no longer applies cleanly. You have to "svn update" to revision 1717026. Then it works and compiles, but updating then to trunk again leads to conflicts and compile errors. It looks like this patch has so many changes so it should be applied ASAP. [~gchanan]'s fix for SolrCore should be applied, too. > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: Trunk > > Attachments: SOLR-8330-combined.patch, SOLR-8330-detector.patch, > SOLR-8330-detector.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch, SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15033892#comment-15033892 ] Jason Gerlowski commented on SOLR-8330: --- Tests passed for me, so I _think_ this patch should be looking good. > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: Trunk > > Attachments: SOLR-8330-combined.patch, SOLR-8330-detector.patch, > SOLR-8330-detector.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15032731#comment-15032731 ] Gregory Chanan commented on SOLR-8330: -- RequestLoggingTest is still relevant -- it checks the output of specific loggers. I don't quite understand why it doesn't work though -- does the MethodHandles change the name of the logger? > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: Trunk > > Attachments: SOLR-8330-combined.patch, SOLR-8330-detector.patch, > SOLR-8330-detector.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch, SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[
https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15032760#comment-15032760
]
Gregory Chanan commented on SOLR-8330:
--
BTW changing the requestLog in SolrCore.java to:
{code} requestLog =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass().getName()+
".Request");{code}
gets the test to pass for me.
> Restrict logger visibility throughout the codebase to private so that only
> the file that declares it can use it
> ---
>
> Key: SOLR-8330
> URL: https://issues.apache.org/jira/browse/SOLR-8330
> Project: Solr
> Issue Type: Sub-task
>Affects Versions: Trunk
>Reporter: Jason Gerlowski
>Assignee: Anshum Gupta
> Labels: logging
> Fix For: Trunk
>
> Attachments: SOLR-8330-combined.patch, SOLR-8330-detector.patch,
> SOLR-8330-detector.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch,
> SOLR-8330.patch, SOLR-8330.patch
>
>
> As Mike Drob pointed out in Solr-8324, many loggers in Solr are
> unintentionally shared between classes. Many instances of this are caused by
> overzealous copy-paste. This can make debugging tougher, as messages appear
> to come from an incorrect location.
> As discussed in the comments on SOLR-8324, there also might be legitimate
> reasons for sharing loggers between classes. Where any ambiguity exists,
> these instances shouldn't be touched.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15032798#comment-15032798 ] Anshum Gupta commented on SOLR-8330: Thanks Greg and Jason. I'll take a look at this patch and commit this after the tests pass, along with Uwe's awesome validator :) > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: Trunk > > Attachments: SOLR-8330-combined.patch, SOLR-8330-detector.patch, > SOLR-8330-detector.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch, SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15033273#comment-15033273 ] Uwe Schindler commented on SOLR-8330: - This is the correct fix. Class#toString() (which is used implicit by the string concat here), returns "class oas.SolrCore" including "class " prefix. This is a common programming error. > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: Trunk > > Attachments: SOLR-8330-combined.patch, SOLR-8330-detector.patch, > SOLR-8330-detector.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch, SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15033277#comment-15033277 ] Uwe Schindler commented on SOLR-8330: - I will try the patch later, but looks fine to me; small issues: - the changes entry is missing a space before the names - the excluded file in the ANT pattern in the checker should be maybe a bit more specific. The only backside with the exclusion here is that it omits all checks on this file, but that's acceptable. The CheckLoggingConfiguration class is not really containing much code... > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: Trunk > > Attachments: SOLR-8330-combined.patch, SOLR-8330-detector.patch, > SOLR-8330-detector.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch, SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[
https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15030688#comment-15030688
]
Jason Gerlowski commented on SOLR-8330:
---
Upon slightly closer inspection, the two interfaces that contain loggers
({{SolrCache}} and {{SolrEventListener}}) aren't actually used anywhere. So
they could/should be deleted.
With that in mind, do we even want to allow interfaces to have loggers for now?
There's a solid rationale when we move to Java8, sure. But I'd argue that
until then, interface-loggers shouldn't be encouraged/allowed. Changing the
source-check to allow loggers in interfaces could be part of the move-to-Java8
effort (as long as we can make sure this doesn't get lost when the move
actually happens...that's my main qualm).
> Restrict logger visibility throughout the codebase to private so that only
> the file that declares it can use it
> ---
>
> Key: SOLR-8330
> URL: https://issues.apache.org/jira/browse/SOLR-8330
> Project: Solr
> Issue Type: Sub-task
>Affects Versions: Trunk
>Reporter: Jason Gerlowski
>Assignee: Anshum Gupta
> Labels: logging
> Fix For: Trunk
>
> Attachments: SOLR-8330-detector.patch, SOLR-8330-detector.patch,
> SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch,
> SOLR-8330.patch
>
>
> As Mike Drob pointed out in Solr-8324, many loggers in Solr are
> unintentionally shared between classes. Many instances of this are caused by
> overzealous copy-paste. This can make debugging tougher, as messages appear
> to come from an incorrect location.
> As discussed in the comments on SOLR-8324, there also might be legitimate
> reasons for sharing loggers between classes. Where any ambiguity exists,
> these instances shouldn't be touched.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15030780#comment-15030780 ] Anshum Gupta commented on SOLR-8330: Sure, sounds good to me. It's reasonable and also helps us move forward at this point. I'm not going to be able to look at the patch for about 16-18 more hours from now but should be back online after that. Jet lagged but online. So, I'll take a look at it then. > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: Trunk > > Attachments: SOLR-8330-detector.patch, SOLR-8330-detector.patch, > SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15030773#comment-15030773 ] Mark Miller commented on SOLR-8330: --- Indeed. We dont need to burn bridges that are not built yet. When we move to Java 8 and someone is dying to add a logger to an interface they can open an issue. > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: Trunk > > Attachments: SOLR-8330-detector.patch, SOLR-8330-detector.patch, > SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15030778#comment-15030778 ] Jason Gerlowski commented on SOLR-8330: --- Cool, If Anshum's fine w/ that plan, I'm happy to update the patch with that in mind (though the changes are pretty trivial). But I'll refrain from making any changes until he's able to chime in. > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: Trunk > > Attachments: SOLR-8330-detector.patch, SOLR-8330-detector.patch, > SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15030157#comment-15030157 ] Erick Erickson commented on SOLR-8330: -- Just alerting you to this if you run into it. > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: Trunk > > Attachments: SOLR-8330-detector.patch, SOLR-8330-detector.patch, > SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15030055#comment-15030055 ] Anshum Gupta commented on SOLR-8330: If you mean the svn merge, hopefully it'll take care of it. :) > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: Trunk > > Attachments: SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15030101#comment-15030101 ] Uwe Schindler commented on SOLR-8330: - Hi, I had an idea a minute ago how to prevent incorrectly declared loggers. In root's build.xml we have the source-patterns ant task (that greps on source files for "violations"). We could add a regular expression for finding "bad loggers" and fail "ant validate" because of that. We can commit this afterwards, I can work on some regexes to detect this (some combinations of positive/negative patterns). > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: Trunk > > Attachments: SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch, SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15030074#comment-15030074 ] Anshum Gupta commented on SOLR-8330: OpenCloseCoreStressTest is another file with multiple classes. I've declared multiple loggers in there but I think we should move away from the practice. Putting up a patch in a bit with the tests fixed as well. > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: Trunk > > Attachments: SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15030088#comment-15030088 ] Uwe Schindler commented on SOLR-8330: - +1 LGTM > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: Trunk > > Attachments: SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch, SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15030093#comment-15030093 ] Jason Gerlowski commented on SOLR-8330: --- Looks good to me; thanks Anshum. > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: Trunk > > Attachments: SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch, SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[
https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15030197#comment-15030197
]
Jason Gerlowski commented on SOLR-8330:
---
I think having a source-patterns check is a good idea in general, but it might
not be as useful as we hope.
For example, a number of the violations in Uwe's comment above are (arguably)
false positives.
1.) Several interfaces contain Logger declarations. These declarations can't
be private since interfaces can't contain private members. These Loggers
should probably be removed entirely. I didn't do this in my initial patch
because I was making an effort to be conservative about what I changed, but I
really don't think there's a reason to keep them around. This affects
{{SolrEventListener}}, and {{SolrCache}}.
2.) Some Java files contain two non-nested classes which share loggers. For
example:
{code:java}
//All code in file A.java
class A {
static final Logger log = ;
}
class B {
static final Logger log = B.log;
}
{code}
Because the classes aren't nested, the logger can't be a private member. The
classes can be made to use separate loggers, but this would break Mike Drob's
point that:
bq: As somebody that looks at customer logs for the majority of my interaction
with Solr, I would want Loggers to correspond to file names every single time.
This corner case affects {{SurroundQParserPlugin}},
{{IgnoreCommitOptimizeUpdateProcessorFactory}}, and
{{LogUpdateProcessorFactory}}.
3.) Some of the Java files reported above are actually testing log4j
configuration/setup. Often, these tests declare Loggers in non-standard ways.
For example {{TestLogWatcher}} declares a Logger as a local variable in a test
method.
To clarify my point a bit, I'm not saying that a we shouldn't have a
source-analysis check for this. But the fact that there are already corner
cases/edge cases probably means this shouldn't be used as pass/fail criteria
for {{ant validate}}. And unfortunately it might mean that the check will be
ignored more often than not (since it doesn't really have any "teeth").
> Restrict logger visibility throughout the codebase to private so that only
> the file that declares it can use it
> ---
>
> Key: SOLR-8330
> URL: https://issues.apache.org/jira/browse/SOLR-8330
> Project: Solr
> Issue Type: Sub-task
>Affects Versions: Trunk
>Reporter: Jason Gerlowski
>Assignee: Anshum Gupta
> Labels: logging
> Fix For: Trunk
>
> Attachments: SOLR-8330-detector.patch, SOLR-8330-detector.patch,
> SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch,
> SOLR-8330.patch
>
>
> As Mike Drob pointed out in Solr-8324, many loggers in Solr are
> unintentionally shared between classes. Many instances of this are caused by
> overzealous copy-paste. This can make debugging tougher, as messages appear
> to come from an incorrect location.
> As discussed in the comments on SOLR-8324, there also might be legitimate
> reasons for sharing loggers between classes. Where any ambiguity exists,
> these instances shouldn't be touched.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15030073#comment-15030073 ] Anshum Gupta commented on SOLR-8330: I couldn't think of an easy way to split it out other than not doing this for the tests or certain parts of the code. I wanted to keep it easy but it's been a rabbit hole. I think it's a ton of volume but not really intrusive in terms of functional impact. What do you think? > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: Trunk > > Attachments: SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15030323#comment-15030323 ] Mark Miller commented on SOLR-8330: --- +1 to Uwe's validator and making the changes for it to pass. Awesome. > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: Trunk > > Attachments: SOLR-8330-detector.patch, SOLR-8330-detector.patch, > SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15030215#comment-15030215 ] Anshum Gupta commented on SOLR-8330: [~gerlowskija] About loggers in interfaces, with Java8, you could write a default implementation for a method as part of the interface declaration, thereby requiring the logger. I know this was brought up a while ago when we decided to move to Java 8 on trunk. > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: Trunk > > Attachments: SOLR-8330-detector.patch, SOLR-8330-detector.patch, > SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15030351#comment-15030351 ] Anshum Gupta commented on SOLR-8330: +1 to Uwe's validator and making those changes but here's my concern with 1. Once we move to Java 8; possibly in the next couple of releases. Once that happens, we might have developers wanting to write default implementation in interfaces and we'd have to fix this then anyways so I'm not sure if we want to remove that for now. > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: Trunk > > Attachments: SOLR-8330-detector.patch, SOLR-8330-detector.patch, > SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15030208#comment-15030208 ] Anshum Gupta commented on SOLR-8330: Thanks Uwe! The only thing that's stopping me is, I have a _trans atlantic_ flight, and more which might keep me offline for about 40 hours later today. I don't want to commit this and be gone so 1. I can either commit it when I'm back online 2. Someone else can commit this and take it from here P.S: The RequestLoggingTest is still failing and I'm not sure if that test makes sense any more or not. [~gchanan], seem like you'd have a better idea about that one. Thoughts? > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: Trunk > > Attachments: SOLR-8330-detector.patch, SOLR-8330-detector.patch, > SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15030236#comment-15030236 ] Uwe Schindler commented on SOLR-8330: - Hi for the other changes: All fixes are quite easy: 1) maybe we should remove loggers from interfaces, without any static methods (in Java 7 there cannot be any), it makes no sense to have them. So put them into the impl class 2) The non-nested classes are an anti-pattern from earlier Java 1.0 days. We should simply remove them (move the parallel class as static inner class into the main class). I did this several times whenever I saw them. Those constructs generally have the problem that it does not work with incremental compile (javac fails to compile incrementally when you change source files, because it cannot corelate the class files to source files). So we should fix them. 3) The tests can get a fake logger or let's exclude them. There is also the CheckLoggingConfiguration in servlet package. This should be excluded from checks (which is quite easy to do). If this is fixed we are fine. > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: Trunk > > Attachments: SOLR-8330-detector.patch, SOLR-8330-detector.patch, > SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[
https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15030233#comment-15030233
]
Jason Gerlowski commented on SOLR-8330:
---
Makes sense, glad I didn't delete them then. That makes it a stronger case for
not failing {{ant validate}} for these violations then.
> Restrict logger visibility throughout the codebase to private so that only
> the file that declares it can use it
> ---
>
> Key: SOLR-8330
> URL: https://issues.apache.org/jira/browse/SOLR-8330
> Project: Solr
> Issue Type: Sub-task
>Affects Versions: Trunk
>Reporter: Jason Gerlowski
>Assignee: Anshum Gupta
> Labels: logging
> Fix For: Trunk
>
> Attachments: SOLR-8330-detector.patch, SOLR-8330-detector.patch,
> SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch,
> SOLR-8330.patch
>
>
> As Mike Drob pointed out in Solr-8324, many loggers in Solr are
> unintentionally shared between classes. Many instances of this are caused by
> overzealous copy-paste. This can make debugging tougher, as messages appear
> to come from an incorrect location.
> As discussed in the comments on SOLR-8324, there also might be legitimate
> reasons for sharing loggers between classes. Where any ambiguity exists,
> these instances shouldn't be touched.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15029443#comment-15029443 ] Erick Erickson commented on SOLR-8330: -- CdcrTransactionLog is not in the 5x code line, so that'll be one "interesting" thing to reconcile. FYI > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: Trunk > > Attachments: SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-8330) Restrict logger visibility throughout the codebase to private so that only the file that declares it can use it
[ https://issues.apache.org/jira/browse/SOLR-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15029441#comment-15029441 ] Mike Drob commented on SOLR-8330: - Yea, I was concerned that there would be some difficulty extracting loggers used in multiple places. I initially envisioned doing all the easy stuff in one JIRA and then taking care of the difficult stuff in separate child tasks as necessary. > Restrict logger visibility throughout the codebase to private so that only > the file that declares it can use it > --- > > Key: SOLR-8330 > URL: https://issues.apache.org/jira/browse/SOLR-8330 > Project: Solr > Issue Type: Sub-task >Affects Versions: Trunk >Reporter: Jason Gerlowski >Assignee: Anshum Gupta > Labels: logging > Fix For: Trunk > > Attachments: SOLR-8330.patch, SOLR-8330.patch, SOLR-8330.patch, > SOLR-8330.patch > > > As Mike Drob pointed out in Solr-8324, many loggers in Solr are > unintentionally shared between classes. Many instances of this are caused by > overzealous copy-paste. This can make debugging tougher, as messages appear > to come from an incorrect location. > As discussed in the comments on SOLR-8324, there also might be legitimate > reasons for sharing loggers between classes. Where any ambiguity exists, > these instances shouldn't be touched. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
