You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-dev@jackrabbit.apache.org by Chetan Mehrotra <ch...@gmail.com> on 2015/07/15 05:10:14 UTC

Re: svn commit: r1690941 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/plugins/document/rdb/ test/java/org/apache/jackrabbit/oak/plugins/document/

Hi Julian,

On Tue, Jul 14, 2015 at 7:57 PM,  <re...@apache.org> wrote:
> +
> +            List<String> ids = new ArrayList<String>();
> +            for (T doc : documents) {
> +                ids.add(doc.getId());
> +            }
> +            LOG.debug("insert of " + ids + " failed", ex);
> +
> +            // collect additional exceptions
> +            String messages = LOG.isDebugEnabled() ? RDBJDBCTools.getAdditionalMessages(ex) : "";
> +            if (!messages.isEmpty()) {
> +                LOG.debug("additional diagnostics: " + messages);
> +            }

If all that work is to be done for debug logging then probably the
whole block should be within isDebugEnabled check

+ LOG.debug("insert of " + ids + " failed", ex);

Instead of using concatenation it would be better to use placeholders like

LOG.debug("insert of {} failed",ids, ex);

Chetan Mehrotra

Re: svn commit: r1690941 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/plugins/document/rdb/ test/java/org/apache/jackrabbit/oak/plugins/document/

Posted by Julian Reschke <ju...@gmx.de>.
On 2015-07-15 05:10, Chetan Mehrotra wrote:
> Hi Julian,
>
> On Tue, Jul 14, 2015 at 7:57 PM,  <re...@apache.org> wrote:
>> +
>> +            List<String> ids = new ArrayList<String>();
>> +            for (T doc : documents) {
>> +                ids.add(doc.getId());
>> +            }
>> +            LOG.debug("insert of " + ids + " failed", ex);
>> +
>> +            // collect additional exceptions
>> +            String messages = LOG.isDebugEnabled() ? RDBJDBCTools.getAdditionalMessages(ex) : "";
>> +            if (!messages.isEmpty()) {
>> +                LOG.debug("additional diagnostics: " + messages);
>> +            }
>
> If all that work is to be done for debug logging then probably the
> whole block should be within isDebugEnabled check
>
> + LOG.debug("insert of " + ids + " failed", ex);
>
> Instead of using concatenation it would be better to use placeholders like
>
> LOG.debug("insert of {} failed",ids, ex);
>
> Chetan Mehrotra

Understood. But keep in mind this is error handling. It's not 
performance relevant at all.

Best regards, Julian