ARTEMIS-1737 fix for inaccessible slave console after failover #1940
Conversation
|
Is this on fail back to master? With restart enabled? If so shouldn’t the console be in the standard logic with the other components? Eg simply things that should be restarted or not shutdown should all be in the same logic block, else we would end up with lots of code branches to maintain to deal with this. Eg network health check Also I believe previous original version the console did remain, checking history it seems a commit on the 29/9/17 changed the isExit flag from false to true possibly causing this, and may have other knock on effects that are similar to the console. Eg I believe network pinger also is affected the same (this is quite serious) @clebertsuconic could you check on this as I think the change for fail method for isExit from false to true was done by yourself 1b0fc06 so you will have better knowledge. Tracing the code and testing it seems it’s not just console that got affected by that change, as noted above. |
|
Yes, it is. I used this conf: https://pastebin.com/2G0c341X |
|
@stanlyDoge its not just the console affected. the root cause of the issue you are seeing was that actually fail() method that you're kinda of cloning in your change used to have the isExit flag to false, there are actually a few places that can be invoked as well, as such whilst the negative effect of this change that ive called out, is most visible with the console, there are more serious bits seen that also look like they're affected such as Network Pinger, e.g. previously it's thread wasn't stopped, but since the isExit boolean change done prior it is. This really needs to have @clebertsuconic look over his original change, as it seems that it is what caused this, also others in my opinion. And any fix needs to not just care about the console. |
|
Ah, I see. Yeah, it seem to be changed in ARTEMIS-1440. @clebertsuconic can you take a look? |
|
@stanlyDoge nice test though!! What ever the fix worth making sure it gets added in :) |
|
I'm running the testsuite... I will merge a modified patch.. thanks a lot @stanlyDoge |
|
@clebertsuconic i assume you will PR your modifications before merge. If you could @ me on the PR, as im away tomorrow on leave, so wont be checking the normal mail list, but @ will mean it goes to my main inbox and ill see it to look at asap. |
|
@michaelandrepearce will do.. I thought I would be able to do it tonight... I'm hitting another issue.. so it will be tomorrow. |
Can anybody review this? I will try to write some tests meanwhile. Thank you.