fix(engine): Skip ByteArray Deletion when Updating Var #4433

psavidis · 2024-06-12T09:06:49Z

Problem Description: During Set Variable and Read Variable operations, the following partial ORM operations can occur in a problematic sequence:

Thread A reads an Object Variable, thus associated with a byte array value
Thread B reads the same Object Variable and updates its value to the db
Now Thread A has a leaking byte array reference that is null

Solution: Instead of deleting an updating the byte array value, during the update, the deletion is simply skipped.
The fix was verified with the customer.

The above behaviour introduces leftover byte-array state on type change. Thus, only during a type change, the byte-array is deleted.

Unit-tests: This commit introduces unit tests that verify the correct state of the variables on type change.

Limitations Accepted: The issue could not be reproduced with a unit test. However, this is accepted since the fix follows a common theme for value update across all the project and is not expected to produce a regression. Moreover, the mere reproduction of the problem using private calls doesn't give any extra value.

Co-authored-by: tasso94, petros.savvidis, yanavasileva
Related-to: #4324, https://jira.camunda.com/browse/SUPPORT-21494

psavidis · 2024-06-12T09:08:02Z

Notes for the Reviewer

The behaviour to skip deleting the ByteArray introduces the side-effect of having a leftover byte-array state when the type changes e.g from Object to Long
To mitigate that problem, the test shouldUpdateVariableStateOnVariableTypeChangeObjectToLong was added to verify that the correct state is populated when the variable has type object, but also when the variable changes to long.
- That test fails on passes on master because on master, each update will always delete the byte-array
- On SUPPORT-21721, the test fails without the fix on type change.

Note: The other two tests for Double-to-Object, String-to-Object type changes were added despite the fact that they do not fail on both branches. It is still important to add that coverage for other types

yanavasileva

👍 I have minor (cosmetic) code improvements below.

engine/src/main/java/org/camunda/bpm/engine/impl/persistence/entity/VariableInstanceEntity.java

engine/src/test/java/org/camunda/bpm/engine/test/api/runtime/VariableInstanceTest.java

psavidis · 2024-06-19T15:35:09Z

Applied all points as suggested.

yanavasileva

👍 Thank you for considering my input. Please wait for the CI before merging.

engine/src/main/java/org/camunda/bpm/engine/impl/persistence/entity/VariableInstanceEntity.java

engine/src/test/java/org/camunda/bpm/engine/test/api/runtime/VariableInstanceTest.java

engine/src/main/java/org/camunda/bpm/engine/impl/persistence/entity/VariableInstanceEntity.java

tasso94

❌ I doubt this change fixes the problem of the customer.

When I look at the following logs, I see that the byte array with this change is still deleted and reinserted:

 09:51:38.596 [main] DEBUG o.c.b.e.i.p.e.V.insertByteArray - ==>  Preparing: insert into ACT_GE_BYTEARRAY(ID_, NAME_, BYTES_, DEPLOYMENT_ID_, TENANT_ID_, TYPE_, CREATE_TIME_, ROOT_PROC_INST_ID_, REMOVAL_TIME_, REV_) values ( ?, ?, ?, ?, ?, ?, ?, ?, ?, 1 )
 09:51:38.597 [main] DEBUG o.c.b.e.i.p.e.V.insertByteArray - ==> Parameters: 16(String), myVar(String), java.io.ByteArrayInputStream@196d684b(ByteArrayInputStream), null, null, 2(Integer), 2024-06-20 09:51:38.586(Timestamp), null, null
 09:51:38.597 [main] DEBUG org.camunda.bpm.engine.persistence - ENGINE-03009 SQL operation: 'INSERT'; Entity: 'ByteArrayEntity[id=17]'
 09:51:38.597 [main] DEBUG o.c.b.e.i.p.e.V.insertByteArray - ==> Parameters: 17(String), myVar(String), java.io.ByteArrayInputStream@45d38165(ByteArrayInputStream), null, null, 3(Integer), 2024-06-20 09:51:38.587(Timestamp), 5(String), null
 09:51:38.597 [main] DEBUG org.camunda.bpm.engine.persistence - ENGINE-03009 SQL operation: 'INSERT'; Entity: 'ByteArrayEntity[id=19]'
 09:51:38.597 [main] DEBUG o.c.b.e.i.p.e.V.insertByteArray - ==> Parameters: 19(String), myVar(String), java.io.ByteArrayInputStream@1f3f434(ByteArrayInputStream), null, null, 3(Integer), 2024-06-20 09:51:38.588(Timestamp), 5(String), null
 09:51:38.597 [main] DEBUG org.camunda.bpm.engine.persistence - ENGINE-03009 SQL operation: 'DELETE'; Entity: 'MessageEntity[id=12]'
 09:51:38.597 [main] DEBUG org.camunda.bpm.engine.persistence - ENGINE-03009 SQL operation: 'UPDATE'; Entity: 'VariableInstanceEntity[id=7]'
 09:51:38.597 [main] DEBUG o.c.b.e.i.p.e.V.updateVariableInstance - ==>  Preparing: update ACT_RU_VARIABLE set REV_ = ?, EXECUTION_ID_ = ?, VAR_SCOPE_ = ?, TYPE_ = ?, BYTEARRAY_ID_ = ?, DOUBLE_ = ?, LONG_ = ?, TEXT_ = ?, TEXT2_ = ?, SEQUENCE_COUNTER_ = ? where ID_ = ? and REV_ = ?
 09:51:38.598 [main] DEBUG o.c.b.e.i.p.e.V.updateVariableInstance - ==> Parameters: 2(Integer), 5(String), 5(String), serializable(String), 16(String), null, null, null, org.camunda.bpm.engine.test.bpmn.scripttask.MySerializable(String), 2(Long), 7(String), 1(Integer)
 09:51:38.598 [main] DEBUG org.camunda.bpm.engine.persistence - ENGINE-03009 SQL operation: 'DELETE'; Entity: 'ByteArrayEntity[id=10]'
 09:51:38.598 [main] DEBUG o.c.b.e.i.p.e.V.deleteByteArray - ==>  Preparing: delete from ACT_GE_BYTEARRAY where ID_ = ? and REV_ = ?
 09:51:38.598 [main] DEBUG o.c.b.e.i.p.e.V.deleteByteArray - ==> Parameters: 10(String), 1(Integer)
 09:51:38.598 [main] DEBUG org.camunda.bpm.engine.persistence - ENGINE-03009 SQL operation: 'DELETE'; Entity: 'ByteArrayEntity[id=6]'
 09:51:38.598 [main] DEBUG o.c.b.e.i.p.e.V.deleteByteArray - ==> Parameters: 6(String), 1(Integer)

I produce these logs with the following test case in ScriptTaskTest:

  @Test
  public void produceLogsForupdateByteArrayBehavior() {
    deployProcess(Bpmn.createExecutableProcess("testProcess")
        .startEvent().camundaAsyncAfter()
        .scriptTask()
        .scriptFormat("groovy")
        .scriptText("println 'var ' + myVar")
        .scriptTask()
        .scriptFormat("groovy")
        .scriptText("execution.setVariable('myVar', new org.camunda.bpm.engine.test.bpmn.scripttask.MySerializable('test2'))")
        .userTask()
        .endEvent()
        .done());

    ProcessInstance pi = runtimeService.startProcessInstanceByKey("testProcess",
        Variables.createVariables().putValue("myVar", new org.camunda.bpm.engine.test.bpmn.scripttask.MySerializable("test")));

    String id = managementService.createJobQuery().singleResult().getId();
    managementService.executeJob(id);
  }

Instead, the logs should look like this:

09:59:21.892 [main] DEBUG o.c.b.e.i.p.e.V.insertByteArray - ==>  Preparing: insert into ACT_GE_BYTEARRAY(ID_, NAME_, BYTES_, DEPLOYMENT_ID_, TENANT_ID_, TYPE_, CREATE_TIME_, ROOT_PROC_INST_ID_, REMOVAL_TIME_, REV_) values ( ?, ?, ?, ?, ?, ?, ?, ?, ?, 1 )
09:59:21.901 [main] DEBUG o.c.b.e.i.p.e.V.insertByteArray - ==> Parameters: 10(String), myVar(String), java.io.ByteArrayInputStream@1d8be7b9(ByteArrayInputStream), null, null, 3(Integer), 2024-06-20 09:59:21.868(Timestamp), 5(String), null
09:59:21.901 [main] DEBUG o.c.b.e.i.p.e.V.insertByteArray - ==> Parameters: 6(String), myVar(String), java.io.ByteArrayInputStream@48277712(ByteArrayInputStream), null, null, 2(Integer), 2024-06-20 09:59:21.856(Timestamp), null, null
09:59:21.901 [main] DEBUG o.c.b.e.i.p.e.V.insertByteArray - ==> Parameters: 8(String), myVar(String), java.io.ByteArrayInputStream@3d53e876(ByteArrayInputStream), null, null, 3(Integer), 2024-06-20 09:59:21.863(Timestamp), 5(String), null
09:59:21.901 [main] DEBUG o.c.b.e.i.p.e.V.insertVariableInstance - ==>  Preparing: insert into ACT_RU_VARIABLE ( ID_, TYPE_, NAME_, PROC_DEF_ID_, PROC_INST_ID_, EXECUTION_ID_, CASE_INST_ID_, CASE_EXECUTION_ID_, TASK_ID_, BATCH_ID_, BYTEARRAY_ID_, DOUBLE_, LONG_, TEXT_, TEXT2_, VAR_SCOPE_, SEQUENCE_COUNTER_, IS_CONCURRENT_LOCAL_, TENANT_ID_, REV_ ) values ( ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 1 )
09:59:21.902 [main] DEBUG o.c.b.e.i.p.e.V.insertVariableInstance - ==> Parameters: 7(String), serializable(String), myVar(String), testProcess:1:3(String), 5(String), 5(String), null, null, null, null, 6(String), null, null, null, org.camunda.bpm.engine.test.bpmn.scripttask.MySerializable(String), 5(String), 1(Long), false(Boolean), null
09:59:43.311 [main] DEBUG o.c.b.e.i.p.e.V.selectVariablesByExecutionId - ==>  Preparing: SELECT RES.*, ( case when RES.TASK_ID_ is not null and RES.EXECUTION_ID_ is not null then EXECUTION.ACT_INST_ID_ when RES.CASE_EXECUTION_ID_ is not null then RES.CASE_EXECUTION_ID_ when EXECUTION.PARENT_ID_ is null and RES.IS_CONCURRENT_LOCAL_ = false then EXECUTION.ID_ when EXECUTION.IS_SCOPE_ = true and EXECUTION.PARENT_ID_ is not null and RES.IS_CONCURRENT_LOCAL_ = false then PARENT_EXECUTION.ACT_INST_ID_ else EXECUTION.ACT_INST_ID_ end ) ACT_INST_ID_ FROM ACT_RU_VARIABLE RES LEFT JOIN ACT_RU_EXECUTION EXECUTION ON RES.EXECUTION_ID_ = EXECUTION.ID_ LEFT JOIN ACT_RU_EXECUTION PARENT_EXECUTION ON EXECUTION.PARENT_ID_ = PARENT_EXECUTION.ID_ WHERE EXECUTION_ID_ = ? AND TASK_ID_ is null
09:59:43.311 [main] DEBUG o.c.b.e.i.p.e.V.selectVariablesByExecutionId - ==> Parameters: 5(String)
09:59:43.318 [main] DEBUG o.c.b.e.i.p.e.V.selectVariablesByExecutionId - <==      Total: 1
09:59:43.318 [main] DEBUG o.c.b.e.i.p.e.V.selectByteArray - ==>  Preparing: select * from ACT_GE_BYTEARRAY where ID_ = ?
09:59:43.318 [main] DEBUG o.c.b.e.i.p.e.V.selectByteArray - ==> Parameters: 6(String)
09:59:43.320 [main] DEBUG o.c.b.e.i.p.e.V.selectByteArray - <==      Total: 1
var org.camunda.bpm.engine.test.bpmn.scripttask.MySerializable@6dde1bf5
09:59:43.363 [main] DEBUG o.c.b.e.i.p.e.V.selectByteArray - ==>  Preparing: select * from ACT_GE_BYTEARRAY where ID_ = ?
09:59:43.364 [main] DEBUG o.c.b.e.i.p.e.V.selectByteArray - ==> Parameters: 10(String)
09:59:43.364 [main] DEBUG o.c.b.e.i.p.e.V.selectByteArray - <==      Total: 1
09:59:43.375 [main] DEBUG o.c.b.e.i.p.e.V.insertByteArray - ==>  Preparing: insert into ACT_GE_BYTEARRAY(ID_, NAME_, BYTES_, DEPLOYMENT_ID_, TENANT_ID_, TYPE_, CREATE_TIME_, ROOT_PROC_INST_ID_, REMOVAL_TIME_, REV_) values ( ?, ?, ?, ?, ?, ?, ?, ?, ?, 1 )
09:59:43.376 [main] DEBUG o.c.b.e.i.p.e.V.insertByteArray - ==> Parameters: 16(String), myVar(String), java.io.ByteArrayInputStream@18dbc1b(ByteArrayInputStream), null, null, 3(Integer), 2024-06-20 09:59:43.363(Timestamp), 5(String), null
09:59:43.376 [main] DEBUG o.c.b.e.i.p.e.V.insertByteArray - ==> Parameters: 18(String), myVar(String), java.io.ByteArrayInputStream@2a510e0e(ByteArrayInputStream), null, null, 3(Integer), 2024-06-20 09:59:43.364(Timestamp), 5(String), null
09:59:43.377 [main] DEBUG o.c.b.e.i.p.e.V.updateVariableInstance - ==>  Preparing: update ACT_RU_VARIABLE set REV_ = ?, EXECUTION_ID_ = ?, VAR_SCOPE_ = ?, TYPE_ = ?, BYTEARRAY_ID_ = ?, DOUBLE_ = ?, LONG_ = ?, TEXT_ = ?, TEXT2_ = ?, SEQUENCE_COUNTER_ = ? where ID_ = ? and REV_ = ?
09:59:43.377 [main] DEBUG o.c.b.e.i.p.e.V.updateVariableInstance - ==> Parameters: 2(Integer), 5(String), 5(String), serializable(String), 6(String), null, null, null, org.camunda.bpm.engine.test.bpmn.scripttask.MySerializable(String), 2(Long), 7(String), 1(Integer)
09:59:43.377 [main] DEBUG o.c.b.e.i.p.e.V.updateByteArray - ==>  Preparing: update ACT_GE_BYTEARRAY set REV_ = ?, BYTES_ = ? where ID_ = ? and REV_ = ?
09:59:43.377 [main] DEBUG o.c.b.e.i.p.e.V.updateByteArray - ==> Parameters: 2(Integer), java.io.ByteArrayInputStream@7c5ac0(ByteArrayInputStream), 6(String), 1(Integer)
09:59:43.377 [main] DEBUG o.c.b.e.i.p.e.V.deleteByteArray - ==>  Preparing: delete from ACT_GE_BYTEARRAY where ID_ = ? and REV_ = ?
09:59:43.377 [main] DEBUG o.c.b.e.i.p.e.V.deleteByteArray - ==> Parameters: 10(String), 1(Integer)

#4433 (review)

psavidis · 2024-06-20T09:28:30Z

❌ I doubt this change fixes the problem of the customer.

When I look at the following logs, I see that the byte array with this change is still deleted and reinserted:

 09:51:38.596 [main] DEBUG o.c.b.e.i.p.e.V.insertByteArray - ==>  Preparing: insert into ACT_GE_BYTEARRAY(ID_, NAME_, BYTES_, DEPLOYMENT_ID_, TENANT_ID_, TYPE_, CREATE_TIME_, ROOT_PROC_INST_ID_, REMOVAL_TIME_, REV_) values ( ?, ?, ?, ?, ?, ?, ?, ?, ?, 1 )
 09:51:38.597 [main] DEBUG o.c.b.e.i.p.e.V.insertByteArray - ==> Parameters: 16(String), myVar(String), java.io.ByteArrayInputStream@196d684b(ByteArrayInputStream), null, null, 2(Integer), 2024-06-20 09:51:38.586(Timestamp), null, null
 09:51:38.597 [main] DEBUG org.camunda.bpm.engine.persistence - ENGINE-03009 SQL operation: 'INSERT'; Entity: 'ByteArrayEntity[id=17]'
 09:51:38.597 [main] DEBUG o.c.b.e.i.p.e.V.insertByteArray - ==> Parameters: 17(String), myVar(String), java.io.ByteArrayInputStream@45d38165(ByteArrayInputStream), null, null, 3(Integer), 2024-06-20 09:51:38.587(Timestamp), 5(String), null
 09:51:38.597 [main] DEBUG org.camunda.bpm.engine.persistence - ENGINE-03009 SQL operation: 'INSERT'; Entity: 'ByteArrayEntity[id=19]'
 09:51:38.597 [main] DEBUG o.c.b.e.i.p.e.V.insertByteArray - ==> Parameters: 19(String), myVar(String), java.io.ByteArrayInputStream@1f3f434(ByteArrayInputStream), null, null, 3(Integer), 2024-06-20 09:51:38.588(Timestamp), 5(String), null
 09:51:38.597 [main] DEBUG org.camunda.bpm.engine.persistence - ENGINE-03009 SQL operation: 'DELETE'; Entity: 'MessageEntity[id=12]'
 09:51:38.597 [main] DEBUG org.camunda.bpm.engine.persistence - ENGINE-03009 SQL operation: 'UPDATE'; Entity: 'VariableInstanceEntity[id=7]'
 09:51:38.597 [main] DEBUG o.c.b.e.i.p.e.V.updateVariableInstance - ==>  Preparing: update ACT_RU_VARIABLE set REV_ = ?, EXECUTION_ID_ = ?, VAR_SCOPE_ = ?, TYPE_ = ?, BYTEARRAY_ID_ = ?, DOUBLE_ = ?, LONG_ = ?, TEXT_ = ?, TEXT2_ = ?, SEQUENCE_COUNTER_ = ? where ID_ = ? and REV_ = ?
 09:51:38.598 [main] DEBUG o.c.b.e.i.p.e.V.updateVariableInstance - ==> Parameters: 2(Integer), 5(String), 5(String), serializable(String), 16(String), null, null, null, org.camunda.bpm.engine.test.bpmn.scripttask.MySerializable(String), 2(Long), 7(String), 1(Integer)
 09:51:38.598 [main] DEBUG org.camunda.bpm.engine.persistence - ENGINE-03009 SQL operation: 'DELETE'; Entity: 'ByteArrayEntity[id=10]'
 09:51:38.598 [main] DEBUG o.c.b.e.i.p.e.V.deleteByteArray - ==>  Preparing: delete from ACT_GE_BYTEARRAY where ID_ = ? and REV_ = ?
 09:51:38.598 [main] DEBUG o.c.b.e.i.p.e.V.deleteByteArray - ==> Parameters: 10(String), 1(Integer)
 09:51:38.598 [main] DEBUG org.camunda.bpm.engine.persistence - ENGINE-03009 SQL operation: 'DELETE'; Entity: 'ByteArrayEntity[id=6]'
 09:51:38.598 [main] DEBUG o.c.b.e.i.p.e.V.deleteByteArray - ==> Parameters: 6(String), 1(Integer)

I produce these logs with the following test case in ScriptTaskTest:

  @Test
  public void produceLogsForupdateByteArrayBehavior() {
    deployProcess(Bpmn.createExecutableProcess("testProcess")
        .startEvent().camundaAsyncAfter()
        .scriptTask()
        .scriptFormat("groovy")
        .scriptText("println 'var ' + myVar")
        .scriptTask()
        .scriptFormat("groovy")
        .scriptText("execution.setVariable('myVar', new org.camunda.bpm.engine.test.bpmn.scripttask.MySerializable('test2'))")
        .userTask()
        .endEvent()
        .done());

    ProcessInstance pi = runtimeService.startProcessInstanceByKey("testProcess",
        Variables.createVariables().putValue("myVar", new org.camunda.bpm.engine.test.bpmn.scripttask.MySerializable("test")));

    String id = managementService.createJobQuery().singleResult().getId();
    managementService.executeJob(id);
  }

Instead, the logs should look like this:

09:59:21.892 [main] DEBUG o.c.b.e.i.p.e.V.insertByteArray - ==>  Preparing: insert into ACT_GE_BYTEARRAY(ID_, NAME_, BYTES_, DEPLOYMENT_ID_, TENANT_ID_, TYPE_, CREATE_TIME_, ROOT_PROC_INST_ID_, REMOVAL_TIME_, REV_) values ( ?, ?, ?, ?, ?, ?, ?, ?, ?, 1 )
09:59:21.901 [main] DEBUG o.c.b.e.i.p.e.V.insertByteArray - ==> Parameters: 10(String), myVar(String), java.io.ByteArrayInputStream@1d8be7b9(ByteArrayInputStream), null, null, 3(Integer), 2024-06-20 09:59:21.868(Timestamp), 5(String), null
09:59:21.901 [main] DEBUG o.c.b.e.i.p.e.V.insertByteArray - ==> Parameters: 6(String), myVar(String), java.io.ByteArrayInputStream@48277712(ByteArrayInputStream), null, null, 2(Integer), 2024-06-20 09:59:21.856(Timestamp), null, null
09:59:21.901 [main] DEBUG o.c.b.e.i.p.e.V.insertByteArray - ==> Parameters: 8(String), myVar(String), java.io.ByteArrayInputStream@3d53e876(ByteArrayInputStream), null, null, 3(Integer), 2024-06-20 09:59:21.863(Timestamp), 5(String), null
09:59:21.901 [main] DEBUG o.c.b.e.i.p.e.V.insertVariableInstance - ==>  Preparing: insert into ACT_RU_VARIABLE ( ID_, TYPE_, NAME_, PROC_DEF_ID_, PROC_INST_ID_, EXECUTION_ID_, CASE_INST_ID_, CASE_EXECUTION_ID_, TASK_ID_, BATCH_ID_, BYTEARRAY_ID_, DOUBLE_, LONG_, TEXT_, TEXT2_, VAR_SCOPE_, SEQUENCE_COUNTER_, IS_CONCURRENT_LOCAL_, TENANT_ID_, REV_ ) values ( ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 1 )
09:59:21.902 [main] DEBUG o.c.b.e.i.p.e.V.insertVariableInstance - ==> Parameters: 7(String), serializable(String), myVar(String), testProcess:1:3(String), 5(String), 5(String), null, null, null, null, 6(String), null, null, null, org.camunda.bpm.engine.test.bpmn.scripttask.MySerializable(String), 5(String), 1(Long), false(Boolean), null
09:59:43.311 [main] DEBUG o.c.b.e.i.p.e.V.selectVariablesByExecutionId - ==>  Preparing: SELECT RES.*, ( case when RES.TASK_ID_ is not null and RES.EXECUTION_ID_ is not null then EXECUTION.ACT_INST_ID_ when RES.CASE_EXECUTION_ID_ is not null then RES.CASE_EXECUTION_ID_ when EXECUTION.PARENT_ID_ is null and RES.IS_CONCURRENT_LOCAL_ = false then EXECUTION.ID_ when EXECUTION.IS_SCOPE_ = true and EXECUTION.PARENT_ID_ is not null and RES.IS_CONCURRENT_LOCAL_ = false then PARENT_EXECUTION.ACT_INST_ID_ else EXECUTION.ACT_INST_ID_ end ) ACT_INST_ID_ FROM ACT_RU_VARIABLE RES LEFT JOIN ACT_RU_EXECUTION EXECUTION ON RES.EXECUTION_ID_ = EXECUTION.ID_ LEFT JOIN ACT_RU_EXECUTION PARENT_EXECUTION ON EXECUTION.PARENT_ID_ = PARENT_EXECUTION.ID_ WHERE EXECUTION_ID_ = ? AND TASK_ID_ is null
09:59:43.311 [main] DEBUG o.c.b.e.i.p.e.V.selectVariablesByExecutionId - ==> Parameters: 5(String)
09:59:43.318 [main] DEBUG o.c.b.e.i.p.e.V.selectVariablesByExecutionId - <==      Total: 1
09:59:43.318 [main] DEBUG o.c.b.e.i.p.e.V.selectByteArray - ==>  Preparing: select * from ACT_GE_BYTEARRAY where ID_ = ?
09:59:43.318 [main] DEBUG o.c.b.e.i.p.e.V.selectByteArray - ==> Parameters: 6(String)
09:59:43.320 [main] DEBUG o.c.b.e.i.p.e.V.selectByteArray - <==      Total: 1
var org.camunda.bpm.engine.test.bpmn.scripttask.MySerializable@6dde1bf5
09:59:43.363 [main] DEBUG o.c.b.e.i.p.e.V.selectByteArray - ==>  Preparing: select * from ACT_GE_BYTEARRAY where ID_ = ?
09:59:43.364 [main] DEBUG o.c.b.e.i.p.e.V.selectByteArray - ==> Parameters: 10(String)
09:59:43.364 [main] DEBUG o.c.b.e.i.p.e.V.selectByteArray - <==      Total: 1
09:59:43.375 [main] DEBUG o.c.b.e.i.p.e.V.insertByteArray - ==>  Preparing: insert into ACT_GE_BYTEARRAY(ID_, NAME_, BYTES_, DEPLOYMENT_ID_, TENANT_ID_, TYPE_, CREATE_TIME_, ROOT_PROC_INST_ID_, REMOVAL_TIME_, REV_) values ( ?, ?, ?, ?, ?, ?, ?, ?, ?, 1 )
09:59:43.376 [main] DEBUG o.c.b.e.i.p.e.V.insertByteArray - ==> Parameters: 16(String), myVar(String), java.io.ByteArrayInputStream@18dbc1b(ByteArrayInputStream), null, null, 3(Integer), 2024-06-20 09:59:43.363(Timestamp), 5(String), null
09:59:43.376 [main] DEBUG o.c.b.e.i.p.e.V.insertByteArray - ==> Parameters: 18(String), myVar(String), java.io.ByteArrayInputStream@2a510e0e(ByteArrayInputStream), null, null, 3(Integer), 2024-06-20 09:59:43.364(Timestamp), 5(String), null
09:59:43.377 [main] DEBUG o.c.b.e.i.p.e.V.updateVariableInstance - ==>  Preparing: update ACT_RU_VARIABLE set REV_ = ?, EXECUTION_ID_ = ?, VAR_SCOPE_ = ?, TYPE_ = ?, BYTEARRAY_ID_ = ?, DOUBLE_ = ?, LONG_ = ?, TEXT_ = ?, TEXT2_ = ?, SEQUENCE_COUNTER_ = ? where ID_ = ? and REV_ = ?
09:59:43.377 [main] DEBUG o.c.b.e.i.p.e.V.updateVariableInstance - ==> Parameters: 2(Integer), 5(String), 5(String), serializable(String), 6(String), null, null, null, org.camunda.bpm.engine.test.bpmn.scripttask.MySerializable(String), 2(Long), 7(String), 1(Integer)
09:59:43.377 [main] DEBUG o.c.b.e.i.p.e.V.updateByteArray - ==>  Preparing: update ACT_GE_BYTEARRAY set REV_ = ?, BYTES_ = ? where ID_ = ? and REV_ = ?
09:59:43.377 [main] DEBUG o.c.b.e.i.p.e.V.updateByteArray - ==> Parameters: 2(Integer), java.io.ByteArrayInputStream@7c5ac0(ByteArrayInputStream), 6(String), 1(Integer)
09:59:43.377 [main] DEBUG o.c.b.e.i.p.e.V.deleteByteArray - ==>  Preparing: delete from ACT_GE_BYTEARRAY where ID_ = ? and REV_ = ?
09:59:43.377 [main] DEBUG o.c.b.e.i.p.e.V.deleteByteArray - ==> Parameters: 10(String), 1(Integer)

Thanks for the detailed test case. I've incorporated and adjusted it into the VariableInstanceTest to have a failing test for the issue with the type change.

The problematic behaviour was the type change detection which was using the class names (the MySerializable was returning Object as a type name).
I've added a test which checks the byteArrayId before and after the update to detect if there's been an delete / insert on the byte array which fails without the fix
The fix uses the class from the value of the variable to make the comparison.

tasso94

❌ Please add the following test cases:

the new value is null, the old value is not null.
the old value is null the new value is not null.

psavidis · 2024-06-20T14:57:58Z

❌ Please add the following test cases:

the new value is null, the old value is not null.

the old value is null the new value is not null.

Added two more test cases as recommended:

Update Variable: null -> not-null (shouldDeleteByteArrayOnUpdateOfExistingNullVariable)
Update Variable: non-null -> null (shouldDeleteByteArrayOnNullifyOfExistingNonNullVariable)

For both test cases, null is considered a separate type, therefore a type change and the byte array should be deleted.

related to SUPPORT-21721

Co-authored-by: yanavasileva <yanavasileva@users.noreply.github.com>

#4433 (review)

yanavasileva

👍 Looks good. I performed some manual concurrency tests, once the byte array is updated, the revision is bumped so a second thread will get the byte array with updated revision instead of returning zero results.

psavidis requested a review from tasso94 June 12, 2024 09:06

psavidis self-assigned this Jun 12, 2024

psavidis added the ci:all-db Runs the builds for all databases. label Jun 12, 2024

psavidis changed the title ~~Support 21721~~ fix(engine): Skip ByteArray Deletion when Updating Var Jun 12, 2024

This was referenced Jun 12, 2024

fix(engine): Skip ByteArray Deletion when Updating Var #4313

Closed

Process variables turn unexpectedly null when jobs are executed in parallel #4324

Closed

psavidis marked this pull request as ready for review June 12, 2024 12:57

psavidis requested review from venetrius and yanavasileva and removed request for tasso94 and venetrius June 18, 2024 08:41

yanavasileva added the ci:default-build Runs the builds that have no explicit trigger (e.g. different history levels). label Jun 19, 2024

yanavasileva reviewed Jun 19, 2024

View reviewed changes

psavidis force-pushed the SUPPORT-21721 branch from 5504e6d to 904ceb7 Compare June 19, 2024 15:34

psavidis requested a review from yanavasileva June 19, 2024 15:34

yanavasileva approved these changes Jun 19, 2024

View reviewed changes

engine/src/main/java/org/camunda/bpm/engine/impl/persistence/entity/VariableInstanceEntity.java Outdated Show resolved Hide resolved

engine/src/test/java/org/camunda/bpm/engine/test/api/runtime/VariableInstanceTest.java Outdated Show resolved Hide resolved

tasso94 reviewed Jun 20, 2024

View reviewed changes

engine/src/main/java/org/camunda/bpm/engine/impl/persistence/entity/VariableInstanceEntity.java Outdated Show resolved Hide resolved

tasso94 requested changes Jun 20, 2024

View reviewed changes

psavidis added a commit that referenced this pull request Jun 20, 2024

Add Test that reproduces the issue

335e2a7

#4433 (review)

psavidis requested a review from tasso94 June 20, 2024 09:30

tasso94 requested changes Jun 20, 2024

View reviewed changes

psavidis requested a review from tasso94 June 20, 2024 14:58

tasso94 and others added 4 commits June 21, 2024 10:04

fix(engine): don't delete byte array deletion when updating var

7b311e9

related to SUPPORT-21721

test(engine): Add Variable Type Change Test

53e5788

test(engine): Delete ByteArray Only on Type Change Only

9570f07

Apply suggestions from code review

c65b080

Co-authored-by: yanavasileva <yanavasileva@users.noreply.github.com>

psavidis and others added 7 commits June 21, 2024 10:04

use simple string comparison

8f125e1

Apply suggestions from code review

80bedc1

Co-authored-by: yanavasileva <yanavasileva@users.noreply.github.com>

Add Test that reproduces the issue

766672a

#4433 (review)

Fix variable type change detection

9be6248

Add null value test cases

4731c2b

Add fix

3a746f0

Fix type name fetching

708c41d

psavidis force-pushed the SUPPORT-21721 branch from cc4c8f2 to 708c41d Compare June 21, 2024 07:04

When setting a variable, don't de-serialize!

74c9803

yanavasileva approved these changes Jun 24, 2024

View reviewed changes

Remove unused import

a65702d

psavidis merged commit 5573dc9 into master Jun 25, 2024
1 of 2 checks passed

psavidis deleted the SUPPORT-21721 branch June 25, 2024 06:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(engine): Skip ByteArray Deletion when Updating Var #4433

fix(engine): Skip ByteArray Deletion when Updating Var #4433

psavidis commented Jun 12, 2024 •

edited

Loading

psavidis commented Jun 12, 2024

yanavasileva left a comment

psavidis commented Jun 19, 2024

yanavasileva left a comment

tasso94 left a comment

psavidis commented Jun 20, 2024 •

edited

Loading

tasso94 left a comment

psavidis commented Jun 20, 2024

yanavasileva left a comment •

edited

Loading

fix(engine): Skip ByteArray Deletion when Updating Var #4433

fix(engine): Skip ByteArray Deletion when Updating Var #4433

Conversation

psavidis commented Jun 12, 2024 • edited Loading

psavidis commented Jun 12, 2024

Notes for the Reviewer

yanavasileva left a comment

Choose a reason for hiding this comment

psavidis commented Jun 19, 2024

yanavasileva left a comment

Choose a reason for hiding this comment

tasso94 left a comment

Choose a reason for hiding this comment

psavidis commented Jun 20, 2024 • edited Loading

tasso94 left a comment

Choose a reason for hiding this comment

psavidis commented Jun 20, 2024

yanavasileva left a comment • edited Loading

Choose a reason for hiding this comment

psavidis commented Jun 12, 2024 •

edited

Loading

psavidis commented Jun 20, 2024 •

edited

Loading

yanavasileva left a comment •

edited

Loading