-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Infinispan can't be manually started with Quarkus 1.3.0 #7910
Comments
This is the Infinispan commit after which it broke: infinispan/infinispan@3dad7af#diff-06d9b67f22d98c28952c53d825f1d4f3R62-R74 |
@geoand I think (didn't check) that then the metrics would just be registered twice, and the second registrations would overwrite the first ones, so while that would be unnecessary, in practice it would work correctly even without fixing the Infinispan part |
That sounds a whole lot better than not being able to start the application at all :) |
Hmm, so in the end, no it actually wouldn't work because there are Gauge metrics, and Gauges can't be re-registered without removing the original Gauge first... In that case I'm getting a different exception
|
OK, so we really need an Infinispan release to fix this issue I guess... We'll also need some tests to make sure this doesn't happen again |
@jmartisk so we could unregister the gauges added by Infinispan to add our own? Are we sure ours will always be registered after the ones from Infinispan? Asking that because I think we need a contingency plan in case we can't get an Infinispan release by the end of the month. |
Also maybe you should open an Infinispan issue describing the problem? |
And another option: if Infinispan now has its own metrics, maybe we should just remove the Quarkus one and let Infinispan do what they think is best for them? |
@gsmet That probably won't work, since Infinispan registers the GC, JVM, etc metrics. I don't think Quarkus users without Infinispan want to live without those metrics. |
This is something wrong that we noticed also and is already fixed in the newly released 10.1.5.Final |
Infinispan embedded not longer does that in 10.1.5.final. Only the infinispan server continues to register those base metrics. |
@gsmet I can try to get a PR with the updated version, but have to get my quarkus env working again :) Should have one soon hopefully, and assuming it passes tests. |
I created #7921 which fixes the client, but I just ran embedded and need to update some xml test files it appears. |
I'm confused about the premise:
is this a supported configuration? Not as far as I know: one should either use the infinispan-embedded extension, or use the server. Incidentally, the infinispan-embedded extension was created by wburns exclusively to make Infinispan Server experiments with Quarkus. Could you tell us more about your use case please? |
@Sanne I think he means he uses the Infinispan client extension and not the embedded one. |
That doesn't seem to be the case. |
The ispn client does not expose MP metrics yet. That can't be the issue. |
@Sanne We create our own EmbeddedCacheManager in a different maven artifact than the one that runs Quarkus (with some logic which defines which configuration file is loaded). This module is then embedded to our Quarkus service, which doesn't know about the Infinispan at all - what it sees is the interfaces to this module which may or may not use Infinispan (offtopic: and might indeed use a mixture of Infinispan and other services to manage the data, although I'm hoping to use Infinispan as a pretty RocksDB wrapper for most use-cases). The other module has no knowledge of Quarkus and it's also used outside Quarkus. It makes a spaghetti code if I have to instantiate my Infinispan inside Quarkus only to start the other module by giving it the cacheManager (thus exposing internal implementation) and then do the same spaghetti code for every other runtime as well. That would pretty much be the end of any good OO principle. |
@burmanm you're bringing up some interesting points, we should probably write up some more about how we expect such designs to evolve; but I won't presume to write any general recommendations yet as many of these are novel ideas: for proper guidance to emerge we'll need more feedback like these and see what works best. To put things into perspective: while this specific problem might have been solved already by #7921 , it's probably best you keep in mind that you're doing something we don't support and don't have integration tests for, so it's possible it will break again without notice. Let me try to explain why and how we expect designs to change - but again I won't presume we have thought of all aspects, so hopefully you'll be able to help figuring this all out. In Quarkus it's fine to use 3rd party libraries which don't have an extension, but in this case:
For most "normal" Java code that people typically develop in house this is not a problem; an extension is only going to be needed for highly dynamic code, complex frameworks, libraries using bytecode enhancement, instrumentation, etc.. So while encapsulation is of course a highly valuable concept, I don't think we can compare instantiation of a complex service like an in-memory data grid cluster component with a straight-forward encapsulation need: such a component needs tight integration with the platform so its initialization, but also integration with management, metrics, operations need to "tie in" into the core platform. Typically one uses a service lookup for such aspects, and you're expected to have your own custom code "lookup" some service which is started and managed by the platform. Wouldn't it be better to use the dependency injection mechanism? Quarkus offers great decoupling of services via CDI - in fact most of our own intregration is relying on it. Offload the initialization and management details of such a component to the platform, and have your component simply inject a reference to the caches. Your other non-Quarkus services can easily provide a simimlar factory, thath's hardly figthing good OO principles. Wouldn't that be better? |
Describe the bug
When launching the application that uses Infinispan directly (not through infinispan-embedded), the startup fails to loading metrics:
This is visible here because of #7907 that causes the displayName to differ from the one that Infinispan loaded, since Infinispan's
new DefaultCacheManager(is)
loads its own metrics before Quarkus:And there the displayName is correct, while with Quarkus it isn't. I'm not sure what changed between 1.2.1.Final and 1.3.0.Final (Infinispan 10.0.0 -> 10.1.2.Final), but now the metrics are loaded multiple times and that breaks things.
Expected behavior
Infinispan would continue working as it did before.
Actual behavior
See above.
To Reproduce
Steps to reproduce the behavior:
Configuration
Screenshots
Additional context
The text was updated successfully, but these errors were encountered: