[JBPM-10197] Handle TimerMappingInfo in error scenarios

[fjtirado]

Thank you for submitting this pull request

JIRA: (please edit the JIRA link if it exists)

link

[martinweiler]

This can lead to duplicate timers, eg. during process instance migration.

[fjtirado]

Ok, I removed transactional check. If ejbTimer is null, it is impossible to remove the job, so the method should return false regardless the transactional status, isnt it?

[martinweiler]

@fjtirado I tested this PR with the following use case:

1. Create process with timer, using postgres DB and oid based schema
2. Manually delete the pg_largeobject record linked to TimerMappingInfo.info to simulate an issue with the retrieval of the info blob from the database.
3. Invoke instances/PROCID/timers API to force reading of timers

Findings: If I remove the throw calls in getEjbTimer method, I can see that the TimerMappingInfo gets recreated, which is what I expected. However, it is missing important information, such as the external id and the info column:

Old TimerMappingInfo entry:

 id |           externaltimerid            | kiesessionid | processinstanceid | timerid |  uuid  | info  
----+--------------------------------------+--------------+-------------------+---------+--------+-------
  6 | 35c5299a-c167-4ca8-bef9-0743a2f35d7b |           13 |                 3 |       2 | 13-3-2 | 18130

New TimerMappingInfo entry:

 id | externaltimerid | kiesessionid | processinstanceid | timerid |  uuid  | info 
----+-----------------+--------------+-------------------+---------+--------+------
  7 |                 |           13 |                 3 |       2 | 13-3-2 |    

[fjtirado]

@martinweiler I think the behaviour you are experiencing (empty externalid and info) is essentially the same that will occur without the patch.
I mean this patch is intended to avoid the performance bottleneck of calling getTimerByName when there is not need to do so , not the problem with empty external id and info.

[sonarcloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

56.6% Coverage
0.0% Duplication

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

[martinweiler]

I'd prefer to have this logged at debug level and also have the jobName added so that the message provides more context

[martinweiler]

obtaining (?)

[albfan]

I try Martin approach to generate the problem.

Definitely, PUT timers when there're two timers active id 1 and id 2, editing id 2 removes from database timer id 1 and id 2 and id 3 refer to second timer but as this was detected on RHPAM 7.12.1, and customer do not have RHPAM-4192 fix, I get a null listing timers again.

I cannot find another action that removes the timer from database, while keeping it in process instance memory. JBPM-10198 should fix that so if that's the only source for "corrupt" a timer that will not happen again. Then this code is defensive for any situation where timer is removed from database.

This is: If there's a timermappinginfo, but timer cannot be retrieved, there's an error so do not look for timer in table, just create another one.

In that sense, code to deal with an error should not be merged, so that's why I offer the alternative PR #2351

If search for timer is not a blocker like implementing https://issues.redhat.com/browse/EAPSUP-1336 (add a column with external id from app using EJB timers) we can ignore this too as search will be really quick.

Just to make it easy to understand:

select 
  id, 
  initial_date, 
  next_date, 
  timer_state,
  encode(decode(info, 'base64'), 'escape') as info,
  (regexp_match(encode(decode(info, 'base64'), 'escape'), '([0-9]+)-([0-9]+)-(.*-[0-9]+)')) as ids, 
  (regexp_match(encode(decode(info, 'base64'), 'escape'), '[0-9]+-([0-9]+)-.*-[0-9]+'))[1] as pid,
  (regexp_match(encode(decode(info, 'base64'), 'escape'), '[-a-z0-9]+_[0-9]+.[0-9]+.[0-9]+')) as container
from jboss_ejb_timer 

This is a query that search in milliseconds what timer is related with what timerID created in RHPAM, but is a hack, we want EJB timers to offer this as a feature

[albfan]

@mareknovotny can we agreed to over #2351 as a solution for customer with this problems instead of this defensive code?

[mareknovotny]

not mine decision, @krisv @fjtirado @martinweiler @gmunozfe are reviewers

[fjtirado]

Im closing this one, will open a new one with alternative approach