Skip to content
This repository has been archived by the owner on Jul 11, 2022. It is now read-only.

Commit

Permalink
[1126853] Alert does not trigger if defined with 'disable when fired'
Browse files Browse the repository at this point in the history
This was undetected fallout from the work done in BZ 1110434.  Although,
it's fallout that doesn't make a lot of sense.  It's either a bug in
HornetQ/Arjuna/EAP interaction Tx management, or we violated some very
subtle J2EE/JTA rule.  Basically this is what was happening:
1) alert condition consumer onMessage is invoked (with default CMT Tx semantics)
2) it invokes a series of nested SLSB calls with various Tx semantics, including
   REQUIRED, REQUIRES_NEW and NOT_SUPPORTED.
3) The NOT_SUPPORTED method hangs on method return, as such some downstream
   processing commits (due to REQUIRES_NEW semantics) but the upstream calls
   don't complete.  So even though the alerting all worked, it never got
   to commit.
4) After 10 minutes the Tx reaper comes along and cancels the hanging Tx.

The fix/workaround is actually easy. Instead of calling the general purpose
disableAlertDefinitions() method (which supports mass-disable and therefore
uses the NOT_SUPPORTED semantics to help chunk transactioning) we switch to
the disableResourceAlertDefinitions() method (which uses REQUIRED) because
we know we are dealing with a single resource-level alert definition.
We make an analogous change for enable.  We are basically avoiding the
issue.

Cherry-pick master: 7362e08
  • Loading branch information
jshaughn committed Aug 6, 2014
1 parent 0a3d6dc commit 212452c
Showing 1 changed file with 9 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1210,9 +1210,11 @@ private void processRecovery(AlertDefinition firedDefinition) {

if (!wasEnabled) {
/*
* recover the other alert, go through the manager layer so as to update the alert cache
* - recover the other alert def, go through the manager layer so as to update the alert cache
* - we know this is a resource alert def so make that call directly. this is more efficient
* and also avoids the issue in BZ 1126853.
*/
alertDefinitionManager.enableAlertDefinitions(overlord, new int[] { recoveryDefinitionId });
alertDefinitionManager.enableResourceAlertDefinitions(overlord, new int[] { recoveryDefinitionId });
}

/*
Expand All @@ -1227,11 +1229,12 @@ private void processRecovery(AlertDefinition firedDefinition) {
}

/*
* disable until recovered manually or by recovery definition
*
* go through the manager layer so as to update the alert cache
* - disable alert def until recovered manually or by recovery definition.
* - go through the manager layer so as to update the alert cache.
* - we know this is a resource alert def so make that call directly. this is more efficient
* and also avoids the issue in BZ 1126853.
*/
alertDefinitionManager.disableAlertDefinitions(overlord, new int[] { firedDefinition.getId() });
alertDefinitionManager.disableResourceAlertDefinitions(overlord, new int[] { firedDefinition.getId() });

/*
* there's no reason to update the cache directly anymore. even though this direct type of update is safe
Expand Down

0 comments on commit 212452c

Please sign in to comment.