Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ISPN-3076 Upgrade to JBoss Logging 3.2.1 #3587

Conversation

tristantarrant
Copy link
Member

@wburns
Copy link
Member

wburns commented Jul 16, 2015

Looks like there are some bugs related to osgi from this.

@tristantarrant
Copy link
Member Author

Updated the PR with much more stuff:

  • ported all custom appenders/filters to the log4j2 api
  • actually run the tests with log4j2
  • upgrade Karaf and Pax Exam to work with log4j2
  • upgrade Hibernate ORM

There is still an issue with the OSGi JPA tests, which I'm trying to resolve with help.

@tristantarrant
Copy link
Member Author

Thanks to @isavin for providing the needed fixes. This is now ready

@danberindei
Copy link
Member

@tristantarrant there's still this warning message I was seeing when I first tried switching to log4j2. Not sure how much of a problem it is:

2015-07-21 19:58:45,981 WARN The Logger org.infinispan.remoting.transport.jgroups.CommandAwareRpcDispatcher was created with the message factory org.apache.logging.log4j.message.ParameterizedMessageFactory@18fb808d and is now requested with the message factory org.apache.logging.log4j.message.StringFormatterMessageFactory@6f84895e, which may create log events with unexpected formatting.

There's also a bad format specifier in AbstractInfinispanTest.ThreadCleaner.close() that was ignored by log4j 1.2, but writes a lot of stack traces to the console with log4j2.

@tristantarrant
Copy link
Member Author

I have fixed AbstractInfinispanTest.ThreadCleaner.close() issue. The other is caused by JGroups attempting to retrieve a logger for MessageDispatcher using LogFactory.getLog(getClass()) which, when using inheritance, means it will request another logger for CommandAwareRpcDispatcher. I'll issue a fix for JGroups.

@danberindei
Copy link
Member

@tristantarrant This hack seems to get rid of the warning, you you think it's worth including it in the PR?

private static final Log log = LogFactory.getLog(CommandAwareRpcDispatcher.Hack.class);

/**
 * Make sure the logger we retrieve with our LogFactory is not the same as the one used by JGroups,
 * because they use JGroups uses a different formatter.
 */
private static class Hack {
}

@tristantarrant
Copy link
Member Author

Nah, let's wait for 3.6.5 which will include belaban/JGroups#211

@danberindei
Copy link
Member

@tristantarrant I ran the test suite again and found a couple more problems:

  1. Using + instead of ,:

    at org.infinispan.distribution.topologyaware.TopologyAwareConsistentHashFactoryTest.testConsistencyWhenNodeLeaves(TopologyAwareConsistentHashFactoryTest.java:427)

  2. Trying to log a mock Throwable from LoggingCallable (used by fork()) throws a NPE:

    at org.infinispan.interceptors.distribution.L1WriteSynchronizerTest.lambda$testSpawnedThreadBlockingException$150(L1WriteSynchronizerTest.java:181)

I don't see any reason why the test couldn't create a proper Exception.

If you fix these 2 and integrate JGroups 3.6.5, the build output should look clean again even with TRACE enabled.

- Also migrate code to use log4j 2.x
- Bump Karaf to 3.0.4
- Bump Pax Exam to 3.5.0
- Update Hibernate dependencies
@tristantarrant
Copy link
Member Author

@danberindei fixed both 1. and 2. above, also fixed another issue in AbstractInfinispanTest where a %i was used in place of %d. As for 3.6.5, we'll just have to wait for it :)

@tristantarrant
Copy link
Member Author

Ignore the CI checks, they were fine before and failed because the agents were reverted to an older JDK. And although I said that we'll have to wait for JGroups 3.6.5, I don't want this PR to be held back by it.

@danberindei
Copy link
Member

Integrated, thanks Tristan!

@tristantarrant tristantarrant deleted the ISPN-3076/jboss_logging_3.2.1 branch December 14, 2015 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants