WFLY-17732 - orphaned JDNI binding on deployment failure #16626
Conversation
github-actions
bot
added
the
deps-ok
|
This fails due to difference in how exported name space work in current WFLY? |
|
BTW @baranowb this is still failing on CI. |
|
Yeah, I saw, no idea why it passed localy... This is just annoying. What is frustrating is that it now pass locally, but I do remember initial failure and another one when I saw this job fail. |
baranowb
force-pushed
the
WFLY-17732
branch
3 times, most recently
from
6a3b7c4
to
a9467e0
Compare
|
@baranowb could you please check CI is still failing? |
soul2zimate
added
hold
|
@parsharma Yeah, Ive seen it. I honestly have no idea, it does not fail locally, nor on anything other than docker image for some reason. |
|
@rhusar PR got updated and CI passed, would you please review ? thanks |
|
I'm not very familiar with naming subsystem. I think this is a question to @emmartins |
|
I will review this during this week |
|
| @@ -301,16 +302,44 @@ protected void addJndiBinding(final EEModuleConfiguration module, final BindingC | |||
| service = (BinderService) controller.getService(); | |||
| if (!equals(service.getSource(), bindingConfiguration.getSource())) { | |||
| throw EeLogger.ROOT_LOGGER.conflictingBinding(bindingName, bindingConfiguration.getSource()); | |||
| } else { | |||
There was a problem hiding this comment.
no matter not being able to replicate the original issue, and having to re-evaluate the JNDI leftovers after deploy fail, this change is not good, it assumes this is about a bind for same deployment yet this is wrong. There are for instance use cases where the same global JNDI is made by multiple deployments, which we should not fail and (unlike this change) skip the increase of the binding ref counter.
There was a problem hiding this comment.
are there any tests for that? if thats a real case, Im fine with ditching this. Nothing fail in TS with his change(AFAIR).
| final ServiceController<ManagedReferenceFactory> binderServiceController = controller; | ||
| final BinderReleaseService binderReleaseService = new BinderReleaseService(binderServiceController, binderService); | ||
| final ServiceBuilder sharedBindingRemovalServiceBuilder = phaseContext.getServiceTarget().addService(phaseContext.getDeploymentUnit().getServiceName().append("sharedBindingReleaseService").append(bindInfo.getBinderServiceName()), binderReleaseService); | ||
| sharedBindingRemovalServiceBuilder.addListener(new LifecycleListener() { |
There was a problem hiding this comment.
this may go in the right direction, since the listener will remove the bind ref on REMOVED, not sure yet about the late acquire tho, I have a feeling that long ago we had something like this and had to change to acquire() immediately. Need more investigation...
|
hmm. indeed something changed in how deployments are handled, thought scanner might have an update: https://issues.redhat.com/browse/WFCORE-6255 but its still not merged. However I can still see leftover bindings: with |
Issue: https://issues.redhat.com/browse/WFLY-17732
Hold on a merge. This needs investigation, as after rebase, behavior changed when it comes to exported JNDI space.