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

PostStart annotation, trace logs on component lifecycle, docs #5468

Merged
merged 5 commits into from
Sep 29, 2017

Conversation

tristantarrant
Copy link
Member

Sorry for the kitchen sink PR...

ISPN-8246 Add a PostStart annotation
- Deprecate AbstractModuleLifecycle and add default methods to ModuleLifecycle
- Invoke modulelifecycle.cacheManagerStarted outside of the GCR start lock
ISPN-8343 Add TRACE logs for component metadata loading and lifecycle invocations
ISPN-8332 Created and Running Cache stats should not count internal caches
ISPN-8352 EmbeddedCacheManager and RemoteCacheManager implement java.io.Closeable
Flesh out CacheManager docs
- Add simple examples of initializing a CacheManager and describe shutdown
- Make configuration a subchapter of the CacheManager chapter

https://issues.jboss.org/browse/ISPN-8246
https://issues.jboss.org/browse/ISPN-8343
https://issues.jboss.org/browse/ISPN-8332
https://issues.jboss.org/browse/ISPN-8352

@tristantarrant tristantarrant added this to the 9.2.0.Alpha1 milestone Sep 27, 2017
@tristantarrant tristantarrant force-pushed the ISPN-8246/poststart branch 2 times, most recently from 8299371 to 684c63d Compare September 28, 2017 07:36
@tristantarrant
Copy link
Member Author

Copy link
Member

@pruivo pruivo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just minor comments

*/
@Deprecated
public class AbstractModuleLifecycle implements ModuleLifecycle {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are still 2 usage in our code:
hibernate-cache: LifecycleCallbacks
core: TestingUtil.addCacheStartingHook()

[source,java]
----

try (EmbeddedCacheManager manager = new DefaultCacheManager()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm... not sure if it is useful

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added Closeable it for symmetry with JCache's CacheManager, but probably documenting it is unnecessary.

@@ -88,9 +89,9 @@ public void testJmxOperationMetadata() throws Exception {
public void testInvokeJmxOperationNotExposed() throws Exception {
try {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could use Exceptions.expectException()

Exceptions.expectException(MBeanException.class, ServiceNotFoundException.class, () -> server.invoke(name, "stop", new Object[]{}, new String[]{}));

@@ -100,14 +101,14 @@ public void testJmxRegistrationAtStartupAndStop(Method m) throws Exception {
CacheContainer otherContainer = TestCacheManagerFactory.createCacheManagerEnforceJmxDomain(otherJmxDomain, true, false);
ObjectName otherName = getCacheManagerObjectName(otherJmxDomain);
try {
assert server.getAttribute(otherName, "CreatedCacheCount").equals("0");
assertEquals("0", server.getAttribute(otherName, "CreatedCacheCount"));
} finally {
otherContainer.stop();
}

try {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could use Exceptions.expectException(). I'm not sure what .equals("0") is doing in the line below

Exceptions.expectException(InstanceNotFoundException.class, () -> server.getAttribute(otherName, "CreatedCacheCount"));

* <p/>
*
* @author Tristan Tarrant
* @since 9.1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

9.2

@tristantarrant
Copy link
Member Author

@pruivo addressed all comments

@pruivo
Copy link
Member

pruivo commented Sep 29, 2017

@tristantarrant there are some checkstyle errors in CI

@tristantarrant
Copy link
Member Author

Yeah, waiting to merge the tx revert so I can rebase and have CI run again

@tristantarrant
Copy link
Member Author

Rebased

- Deprecate AbstractModuleLifecycle and add default methods to ModuleLifecycle
- Invoke modulelifecycle.cacheManagerStarted outside of the GCR start lock
- Add simple examples of initializing a CacheManager and describe shutdown
- Make configuration a subchapter of the CacheManager chapter
@pruivo pruivo merged commit e09a4e3 into infinispan:master Sep 29, 2017
@pruivo
Copy link
Member

pruivo commented Sep 29, 2017

integrated! thanks @tristantarrant

@tristantarrant tristantarrant deleted the ISPN-8246/poststart branch April 10, 2018 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants