Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Persist task id in ACT_HI_VARINST table for task variables #4578

Closed
1 task
jyotisahu9 opened this issue Sep 4, 2024 · 18 comments
Closed
1 task

Persist task id in ACT_HI_VARINST table for task variables #4578

jyotisahu9 opened this issue Sep 4, 2024 · 18 comments
Assignees
Labels
type:feature Issues that add a new user feature to the project.

Comments

@jyotisahu9
Copy link
Contributor

jyotisahu9 commented Sep 4, 2024

User Story (Required on creation)

As a Operations engineer, I would like to know all variables related to specific task

Functional Requirements (Required before implementation)

  • To be able to query all respective task variables using task id from history ACT_HI_VARINST tables

Technical Requirements (Required before implementation)

  • Save task id in existing TASK_ID_ column in history ACT_HI_VARINST table while persisting task variables (input variable or variable created through task or execution listener). Do not update column value if the variable belongs to process instance or any other component.

Pull requests

Preview Give feedback
  1. jyotisahu9
@jyotisahu9 jyotisahu9 added the type:feature Issues that add a new user feature to the project. label Sep 4, 2024
@psavidis psavidis self-assigned this Sep 6, 2024
@psavidis
Copy link
Contributor

psavidis commented Sep 6, 2024

Hi @jyotisahu9,

Could you explain further the Technical Requirements section?

It is not clear to me what is described there as well how it ties to the functional requirements section (being able to query all tasks variables from runtime and history tables)

@jyotisahu9
Copy link
Contributor Author

jyotisahu9 commented Sep 6, 2024

Hi @psavidis,

There is TASK_ID_ column in ACT_HI_VARINST table that remains null for task variable ( created through Execution listeners or task listeners). We would like to reuse this column to save task id to identify these variables related to specific task and query them.

There is VAR_SCOPE_ column in ACT_RU_VARIABLE table that persists the task id or process instance id to identify specific variable, but once that task is completed, entries are deleted from table.

Hope this clarifies the requirement. Let me know !!

Thanks,
Jyoti

@jyotisahu9
Copy link
Contributor Author

Hi @psavidis ,

Does this looks good to you? Do you have any questions?

Thanks,
Jyoti

@psavidis
Copy link
Contributor

Hello @jyotisahu9,
I'm currently occupied on items of high priority for the upcoming release.

I think i understood the requirement, the description / formatting looked a bit confusing to me.

In the meantime, could you break down the technical description into a list of high-level steps that describe what needs to be added? You could for example break it down into layers and mention the table (as you did), the query, the service involved, rest-api etc.

I will review it once i have time and make any necessary adjustments if needed. The goal is to have a technical description which reflects on a high-level scale what needs to be done before starting to work.

Are you going to work on this item?

@jyotisahu9 jyotisahu9 changed the title Persist task id in ACT_HI_VARINST and ACT_RU_VARIABLE tables for task variables Persist task id in ACT_HI_VARINST table for task variables Sep 23, 2024
@jyotisahu9
Copy link
Contributor Author

jyotisahu9 commented Sep 25, 2024

Hi @psavidis

Yes, I will be working on this task.

I would be making the changes to persist taskId in ACT_HI_VARINST table for following two kind of variables:
1. Task Input variables
2. Task Listener variables

Whenever a task element executes( be it through process instance or standalone task), following steps takes place:

  1. createHistoricVariableEvent - Task input variables are created
  2. createActivityInstanceStartEvt
  3. createTaskInstanceCreateEvt
  4. createActivityInstanceUpdateEvt
  5. createTaskInstanceUpdateEvt - Task listener variables are created

I would be plugging in code to set taskId for HistoricVariableInstanceEntity at below steps:

  1. createHistoricVariableEvent - Task input variables are created, but taskId is not generated.
  2. createActivityInstanceStartEvt
  3. createTaskInstanceCreateEvt - Task Id is created at this step
  4. createActivityInstanceUpdateEvt - As all the HistoricVariableInstanceEntities created in step1 are in cache at this point, retrieve them from cache, update taskId for all HistoricVariableInstanceEntities whose activityInstanceId matches with current execution activityInstanceId of task.
  5. createTaskInstanceUpdateEvt - Task listener variables are created, task id is already created at this point so set the task id for HistoricVariableInstanceEntity.

API to start task are:

  1. Through process instance is : /process-definition/key/{key}/start
  2. As a Standalone task : /task/create

API to retrieve task historic variables where taskId will be returned as response parameter are:
Get Variable Instances : /history/variable-instance
Get Variable Instance : /history/variable-instance/{id}

@jyotisahu9
Copy link
Contributor Author

jyotisahu9 commented Oct 8, 2024

Hi @psavidis,

Request you to go through details. Also this is more of bug than feature, I initially created it as feature but then could not change the category.

I have my PR ready that can be pushed to understand the whole implementation.

Thanks,
Jyoti

@jyotisahu9
Copy link
Contributor Author

Hi @psavidis ,

Could you go over the pull request and issue description when you get a chance.

Thanks,
Jyoti

@psavidis
Copy link
Contributor

psavidis commented Nov 5, 2024

Hello @jyotisahu9 ,
Apologies for the late response, it's been quite busy lately.

I'll try to find some time during this week and review the ticket and PR properly.

Till then, a question: Why do you think its a bug?

Best,
Petros

@jyotisahu9
Copy link
Contributor Author

Hi @psavidis,

Thanks for your response.

I think its a bug because there is TASK_ID_ column in ACT_HI_VARINST table but it remains null for task variable. This table saves all variable information and shall persist task id to identify specific task variable

Best,
Jyoti

@psavidis
Copy link
Contributor

psavidis commented Nov 15, 2024

Hello @jyotisahu9 ,

I had a look at the feature request and your pull request and i'd like to share with you my scepticism and questions.

So far, i'm not entirely sure what is the intent of this feature request.
You outlined in a elaborate way what you're going to change but didn't specify why.

It seems to me from the pull request of your contribution there is some confusion about the concept of variables and taskId.

Observation

I've executed the testTaskIdInHistoricVariableInstance (ref) from the PR and the test passes on master.

  • The test should fail if it would cover a missing behaviour (the taskId is not set) but it doesn't fail. Instead it passes succesfully.
  • Can you point me to a test that passes in your branch and fails on master and use that as a reference to explain what's the behaviour you'd like to achieve?

Documentation

If you read the official documentation around process variables, you'll notice the variables can have different scope executions.
Some of these executions concern process-instances and others are related to tasks.

Using that as a reference, i'd like to understand better:

a) What is the problem you'd like to solve
b) Why is that a problem

Best,
Petros

@psavidis
Copy link
Contributor

This thread does not have a response for over a month now on clarifying the feature requirements.
Closing the ticket.

@camunda camunda deleted a comment from github-actions bot Dec 19, 2024
@jyotisahu9
Copy link
Contributor Author

Hi @psavidis ,

Apologies for the delay. I was on vacation, so wasn't able to respond timely.
I will be responding soon.

Thanks,
Jyoti

@jyotisahu9
Copy link
Contributor Author

jyotisahu9 commented Dec 30, 2024

Hello @jyotisahu9 ,

I had a look at the feature request and your pull request and i'd like to share with you my scepticism and questions.

So far, i'm not entirely sure what is the intent of this feature request. You outlined in a elaborate way what you're going to change but didn't specify why.

It seems to me from the pull request of your contribution there is some confusion about the concept of variables and taskId.

Observation

I've executed the testTaskIdInHistoricVariableInstance (ref) from the PR and the test passes on master.

  • The test should fail if it would cover a missing behaviour (the taskId is not set) but it doesn't fail. Instead it passes succesfully.
  • Can you point me to a test that passes in your branch and fails on master and use that as a reference to explain what's the behaviour you'd like to achieve?

None of the test case fail in master branch. I am adding new feature, hence I added additional test cases to cover those.

Documentation

If you read the official documentation around process variables, you'll notice the variables can have different scope executions. Some of these executions concern process-instances and others are related to tasks.

Yes I have gone through scope of variables and I am not changing the scope of task or process variables through my code changes.

Using that as a reference, i'd like to understand better:

a) What is the problem you'd like to solve b) Why is that a problem

I am trying to save task id for task variables (task input variable or task execution variable) in ACT_HI_VARINST table, so that variables specific to task id can be identified. The two api's mentioned below will be then returning the saved task id as response parameter. Table act_hi_detail does save some more details about variables but this table has lot many other entries than variables. Also api's to Get Variable Instances from history table : /history/variable-instance & /history/variable-instance/{id} returns the data as per ACT_HI_VARINST table.

The main goal for this feature request is to save task id in ACT_HI_VARINST table to identify task variables and return them as part of history variable instance api. We would also be using ACT_HI_VARINST table to generate report for all the variables associated with specific task through queries.

Best, Petros

Hi @psavidis

Appreciate your detailed response & patience.

I have tried my best to explain the review comments. Let me know if you have further questions.

Thanks,
Jyoti

@psavidis
Copy link
Contributor

Hello @jyotisahu9 ,

I've read your response and would like to answer it further so that no questions are left unanswered.

None of the test case fail in master branch. I am adding new feature, hence I added additional test cases to cover those.

Initially in the thread you answered your contribution addresses a bug right ?

I've cherry-picked your changes in a branch based on master and comment out the code changes added in DefaultHistoryEventProducer.

The test you've added on HistoricVariableInstanceTest still passes on master without the code changes.
If it was a bug, it should fail on master and pass on your branch. However, that is not the case. The tests passes both on master and on your branch and it's not clear to me what it is testing.

I am trying to save task id for task variables (task input variable or task execution variable) in ACT_HI_VARINST table, so that variables specific to task id can be identified. The two api's mentioned below will be then returning the saved task id as response parameter. Table act_hi_detail does save some more details about variables but this table has lot many other entries than variables. Also api's to Get Variable Instances from history table : /history/variable-instance & /history/variable-instance/{id} returns the data as per ACT_HI_VARINST table.

The main goal for this feature request is to save task id in ACT_HI_VARINST table to identify task variables and return them as part of history variable instance api. We would also be using ACT_HI_VARINST table to generate report for all the variables associated with specific task through queries.

It's clear to me you're interested to use the Variable APIs to fetch variables related to tasks and use them for reporting purposes. That answers what is the original intent of your use case.

What is still fuzzy to me is the problem you're trying to solve as it hasn't been demonstrated yet in a straightforward way.

The Variable History Entries can have null taskId if a variable is not associated to a task. Examples can be variables associated to the whole process-instance, variables associated to call-activities etc. Thus, it's perfectly fine to have entries in history that have a null taskId.

In the functional requirements you state you're interested to query all task variables using the taskId from the History Variables.

Why isn't that the case now? Can you provide a simple example where this does not currently happen using a simple BPMN process definition and a process instance ?

If such an example can't be found, this thread can be considered as closed.

PS: You can still create other issue requests for the team and it would be very helpful if ambiguity can be avoided by demonstrating a problem with examples that have simple steps to reproduce or a feature that has a clear request by specifying what is the problem it overcomes or what is exactly the desired behaviour.

Kind Regards,
Petros

@jyotisahu9
Copy link
Contributor Author

Hi @psavidis ,

Thanks for your descriptive response.

I have tried my best to respond to all of your queries by attaching sample BPMN and important table data before and after my code implementation.

Hello @jyotisahu9 ,

I've read your response and would like to answer it further so that no questions are left unanswered.

None of the test case fail in master branch. I am adding new feature, hence I added additional test cases to cover those.

Initially in the thread you answered your contribution addresses a bug right ?

I've cherry-picked your changes in a branch based on master and comment out the code changes added in DefaultHistoryEventProducer.

The test you've added on HistoricVariableInstanceTest still passes on master without the code changes. If it was a bug, it should fail on master and pass on your branch. However, that is not the case. The tests passes both on master and on your branch and it's not clear to me what it is testing.

I am trying to save task id for task variables (task input variable or task execution variable) in ACT_HI_VARINST table, so that variables specific to task id can be identified. The two api's mentioned below will be then returning the saved task id as response parameter. Table act_hi_detail does save some more details about variables but this table has lot many other entries than variables. Also api's to Get Variable Instances from history table : /history/variable-instance & /history/variable-instance/{id} returns the data as per ACT_HI_VARINST table.
The main goal for this feature request is to save task id in ACT_HI_VARINST table to identify task variables and return them as part of history variable instance api. We would also be using ACT_HI_VARINST table to generate report for all the variables associated with specific task through queries.

It's clear to me you're interested to use the Variable APIs to fetch variables related to tasks and use them for reporting purposes. That answers what is the original intent of your use case.

What is still fuzzy to me is the problem you're trying to solve as it hasn't been demonstrated yet in a straightforward way.

The Variable History Entries can have null taskId if a variable is not associated to a task. Examples can be variables associated to the whole process-instance, variables associated to call-activities etc. Thus, it's perfectly fine to have entries in history that have a null taskId.

Completely agree that taskId would have null value for variables not associated with task. Although in current scenario, taskId columm in act_hi_varinst table is null even for variables associated with task. For attached sample BPMN - task variables like ReviewWorkItem, firstName etc have null task_id value, which restricts me to retrieve all variables associated with task.

In the functional requirements you state you're interested to query all task variables using the taskId from the History Variables.

Why isn't that the case now? Can you provide a simple example where this does not currently happen using a simple BPMN process definition and a process instance ?

Sure, attached sample BPMN and table analysis as requested.

If such an example can't be found, this thread can be considered as closed.

PS: You can still create other issue requests for the team and it would be very helpful if ambiguity can be avoided by demonstrating a problem with examples that have simple steps to reproduce or a feature that has a clear request by specifying what is the problem it overcomes or what is exactly the desired behaviour.

Kind Regards, Petros

Surely will take this into consideration. Hopefully my response helps.

Let me know if any more questions :)

Thanks,
Jyoti

TaskVariables.xlsx

@jyotisahu9
Copy link
Contributor Author

jyotisahu9 commented Jan 23, 2025

Unable to add bpmn as its not allowed file type. Attached bpmn screenshot.

Image

Image

@psavidis
Copy link
Contributor

Hello @jyotisahu9,

I started to recreate the process you posted on the screenshot but i'm afraid there might be discrepancies or slight diverge from your scenario.
Hence, i'd like to kindly ask you top post the bpmn file instead? Perhaps you can upload it to a link provider or commit it in your Pull Request?

Also please state sequentially what steps to follow with the process instance i'll spin up and the expectation that is not met in the end. For instance:

  1. Deploy the process to a process engine (version: ..., distro or other important info for the env)
  2. Start process instance ...
  3. Complete the User Task ReviewWorkItem
  4. Call the endpoint of the history API ... to fetch the variable x.
  5. The History entry of variable x should be populated but its not
    etc.

It would be highly appreciated if the steps to reproduce the problem are simple as the example above. Like that we'll have a common reference of how to reproduce the problem.
It will speed up the analysis and understanding of the issue.

PS: I had a look at the excel sheet but it's not easing the investigation so let's proceed with the above recommendation.

Best,
Petros

@jyotisahu9
Copy link
Contributor Author

Hi @psavidis,

I have attached the bpmn file here https://gist.github.com/jyotisahu9/eda8978ec94e24ce2615c2bee887561d

Let me know if you are able to access it.

Steps mentioned above to reproduce the problem are correct, adding few more details:

  1. Deploy the process to a process engine - version 7.21 or 7.22
  2. Start process instance
  3. Complete the User Task ReviewWorkItem
  4. Call the endpoint of the history API to fetch the variable - ReviewWorkItem, firsName, lastName & taskListenerVariable
    GET /history/variable-instance?variableName=my_variable or
    /history/variable-instance?processInstanceId=current_proc_inst_id
    Current behaviour: response parameter task_id is null
    Expected behaviour: task_id is populated with current task id
  5. Query the database for select * from ACT_HI_VARINST where proc_inst_id_ = current_proc_inst_id
    Check task_id_ column value for above listed task variable, which should be populated but is null

Thanks,
Jyoti

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature Issues that add a new user feature to the project.
Projects
None yet
Development

No branches or pull requests

2 participants