Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 18 additions & 15 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -397,32 +397,32 @@ task.completed event: taskExecutionId = 97469968-64b5-3cdd-b809-e58e4b6e3be7
- ON CONFLICT: `(task_execution_id)` → never matches
- Result: Always INSERTs new row (4 rows for 2 tasks)

**Solution (V2 Migration):**
**Current Solution:**
- Primary key: `(instance_id, task_position)` - composite key
- ON CONFLICT: `(instance_id, task_position)` → matches on same task
- Result: UPDATEs existing row on subsequent events (2 rows for 2 tasks)
- **No task_execution_id column** - ID is derived from composite key

**Rationale:**
- `(instance_id, task_position)` uniquely identifies a task execution within a workflow
- Same workflow + same position = same task (regardless of changing taskExecutionId)
- Enables proper UPSERT behavior despite Quarkus Flow's ID generation
- TaskExecution.id is derived as `instanceId + ":" + taskPosition`

**Migration:** `V2__fix_task_instances_composite_key.sql`
- Drops old PK on `task_execution_id`
- Adds composite PK on `(instance_id, task_position)`
- Updates trigger function to use composite key for conflict resolution
- Cleans up existing duplicates (keeps latest by `last_event_time`)
- Adds index on `task_execution_id` for GraphQL queries by ID
**Schema (V1):**
- Composite PK on `(instance_id, task_position)`
- Trigger function uses composite key for conflict resolution
- No task_execution_id column (derived in application layer)
Comment on lines +412 to +415

**DO:**
- ✅ Use composite key `(instance_id, task_position)` for task identity
- ✅ Keep `task_execution_id` as a regular column (still exposed in GraphQL)
- ✅ Derive TaskExecution.id from `instanceId + ":" + taskPosition`
- ✅ Trust `task_position` to be stable across task lifecycle

**DON'T:**
- ❌ Don't rely on `taskExecutionId` for UPSERT logic (it changes per event)
- ❌ Don't add task_execution_id column to database
- ❌ Don't rely on Quarkus Flow's taskExecutionId for identity (it changes per event)
- ❌ Don't assume Quarkus Flow will fix this (working as designed for their use case)
- ❌ Don't remove `task_execution_id` column (GraphQL API still needs it)

### Metrics & Observability

Expand Down Expand Up @@ -474,16 +474,19 @@ data-index.metrics.transform.poll-interval=30s

**Database columns → JPA Entity → GraphQL:**
```
task_instances.task_execution_id → TaskInstanceEntity.taskExecutionId → TaskExecution.id
task_instances.start → TaskInstanceEntity.start → TaskExecution.start (@JsonProperty("startDate"))
task_instances.end → TaskInstanceEntity.end → TaskExecution.end (@JsonProperty("endDate"))
task_instances.input → TaskInstanceEntity.input → TaskExecution.input (@Ignore, exposed via getInputData())
task_instances.instance_id → TaskInstanceEntity.instanceId → TaskExecution.instanceId
task_instances.task_position → TaskInstanceEntity.taskPosition → TaskExecution.taskPosition
(derived ID) → TaskInstanceEntity.getId() → TaskExecution.getId() (returns "instanceId:taskPosition")
task_instances.start → TaskInstanceEntity.start → TaskExecution.start (@JsonProperty("startDate"))
task_instances.end → TaskInstanceEntity.end → TaskExecution.end (@JsonProperty("endDate"))
task_instances.input → TaskInstanceEntity.input → TaskExecution.input (@Ignore, exposed via getInputData())
```

**IMPORTANT:**
- TaskExecution.id is derived from instanceId + taskPosition (no database column)
- Database uses `start`/`end` (reserved SQL keywords, quoted)
- GraphQL uses `startDate`/`endDate` (via `@JsonProperty`)
- Never use old names: `enter`/`exit`, `triggerTime`/`leaveTime`
- Never use old names: `enter`/`exit`, `triggerTime`/`leaveTime`, `taskExecutionId`

### Error Handling

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,6 @@ public void processBatch(List<TaskExecution> events) {
log.debug("Processing task batch size: {}", events.size());

try {
for (TaskExecution event : events) {
Objects.requireNonNull(event, "event cannot be null");
event.setId(generateTaskExecutionId(event));
}

this.taskPersistence.persistBatch(events);

log.debug("Successfully processed task batch size: {}", events.size());
Expand All @@ -68,10 +63,4 @@ public void processBatch(List<TaskExecution> events) {
throw new ProcessEventFailedException("Failed to process task event batch", e);
}
}

private String generateTaskExecutionId(TaskExecution taskExecutionEvent) {
return taskExecutionEvent.getInstanceId()
+ ":"
+ taskExecutionEvent.getTaskPosition();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -140,33 +140,31 @@ private void tryCreatePlaceholder(TaskExecution event, Connection conn) throws S

private void setTaskParameters(PreparedStatement stmt, TaskExecution event) throws SQLException {

stmt.setString(1, event.getId());
stmt.setString(2, event.getInstanceId());
stmt.setString(3, event.getTaskName());
stmt.setString(4, event.getTaskPosition());
stmt.setString(5, event.getStatus());
stmt.setObject(6, Optional.ofNullable(event.getStart()).map(ZonedDateTime::toOffsetDateTime).orElse(null));
stmt.setObject(7, Optional.ofNullable(event.getEnd()).map(ZonedDateTime::toOffsetDateTime).orElse(null));
stmt.setString(8, toJsonString(event.getInput()));
stmt.setString(9, toJsonString(event.getOutput()));

stmt.setString(1, event.getInstanceId());
stmt.setString(2, event.getTaskName());
stmt.setString(3, event.getTaskPosition());
stmt.setString(4, event.getStatus());
stmt.setObject(5, Optional.ofNullable(event.getStart()).map(ZonedDateTime::toOffsetDateTime).orElse(null));
stmt.setObject(6, Optional.ofNullable(event.getEnd()).map(ZonedDateTime::toOffsetDateTime).orElse(null));
stmt.setString(7, toJsonString(event.getInput()));
stmt.setString(8, toJsonString(event.getOutput()));

// Error fields
if (event.getError() != null) {
stmt.setString(10, event.getError().getType());
stmt.setString(11, event.getError().getTitle());
stmt.setString(12, event.getError().getDetail());
stmt.setObject(13, event.getError().getStatus());
stmt.setString(14, event.getError().getInstance());
stmt.setString(9, event.getError().getType());
stmt.setString(10, event.getError().getTitle());
stmt.setString(11, event.getError().getDetail());
stmt.setObject(12, event.getError().getStatus());
stmt.setString(13, event.getError().getInstance());
} else {
stmt.setNull(9, java.sql.Types.VARCHAR);
stmt.setNull(10, java.sql.Types.VARCHAR);
stmt.setNull(11, java.sql.Types.VARCHAR);
stmt.setNull(12, java.sql.Types.VARCHAR);
stmt.setNull(13, java.sql.Types.INTEGER);
stmt.setNull(14, java.sql.Types.VARCHAR);
stmt.setNull(12, java.sql.Types.INTEGER);
stmt.setNull(13, java.sql.Types.VARCHAR);
}

stmt.setObject(15, Optional.ofNullable(event.getEventTimestamp()).map(ZonedDateTime::toOffsetDateTime).orElse(null));
stmt.setObject(14, Optional.ofNullable(event.getEventTimestamp()).map(ZonedDateTime::toOffsetDateTime).orElse(null));
}

private String toJsonString(JsonNode node) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
INSERT INTO task_instances (
task_execution_id, instance_id, task_name, task_position, status,
instance_id, task_name, task_position, status,
start, "end", input, output,
error_type, error_title, error_detail, error_status, error_instance,
last_event_time, created_at, updated_at
) VALUES (?, ?, ?, ?, ?, ?, ?, ?::jsonb, ?::jsonb, ?, ?, ?, ?, ?, ?, NOW(), NOW())
) VALUES (?, ?, ?, ?, ?, ?, ?::jsonb, ?::jsonb, ?, ?, ?, ?, ?, ?, NOW(), NOW())
ON CONFLICT (instance_id, task_position) DO UPDATE SET
instance_id = COALESCE(EXCLUDED.instance_id, task_instances.instance_id),
task_name = COALESCE(EXCLUDED.task_name, task_instances.task_name),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@
import org.kubesmarts.logic.dataindex.model.TaskExecution;
import org.kubesmarts.logic.dataindex.model.WorkflowInstance;
import org.kubesmarts.logic.dataindex.model.WorkflowInstanceStatus;
import java.time.OffsetDateTime;
import java.util.UUID;

public final class Mapper {

private Mapper() {}
Expand Down Expand Up @@ -91,7 +88,6 @@ public static TaskExecution mapTaskExecutionEvent(CloudEvent cloudEvent, TaskCED
taskExecution.setInstanceId(instanceId);
taskExecution.setEventTimestamp(cloudEvent.getTime().toZonedDateTime());
taskExecution.setStatus(status);
taskExecution.setId(generateTaskExecutionId(instanceId, taskPosition, cloudEvent.getTime()));
taskExecution.setTaskPosition(taskPosition);
taskExecution.setTaskName(taskPosition.substring(taskPosition.lastIndexOf("/") + 1));

Expand Down Expand Up @@ -141,9 +137,4 @@ private static Error mapError(WorkflowError workflowError) {
error.setInstance(workflowError.instance());
return error;
}

private static String generateTaskExecutionId(String instanceId, String taskPosition, OffsetDateTime time) {
String composite = instanceId + ":" + taskPosition + ":" + time.toInstant().toEpochMilli();
return UUID.nameUUIDFromBytes(composite.getBytes()).toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ public void setupTestData() throws Exception {
List<TaskInstanceEntity> tasks1 = new ArrayList<>();

TaskInstanceEntity task1 = new TaskInstanceEntity();
task1.setTaskExecutionId("task-1-1");
task1.setInstanceId(TEST_WORKFLOW_ID_1);
task1.setTaskName("validateInput");
task1.setTaskPosition("/do/0");
Expand All @@ -107,7 +106,6 @@ public void setupTestData() throws Exception {
tasks1.add(task1);

TaskInstanceEntity task2 = new TaskInstanceEntity();
task2.setTaskExecutionId("task-1-2");
task2.setInstanceId(TEST_WORKFLOW_ID_1);
task2.setTaskName("processData");
task2.setTaskPosition("/do/1");
Expand Down Expand Up @@ -144,7 +142,6 @@ public void setupTestData() throws Exception {
List<TaskInstanceEntity> tasks2 = new ArrayList<>();

TaskInstanceEntity task3 = new TaskInstanceEntity();
task3.setTaskExecutionId("task-2-1");
task3.setInstanceId(TEST_WORKFLOW_ID_2);
task3.setTaskName("failingTask");
task3.setTaskPosition("/do/0");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,25 @@ public class TaskExecution {
*/
private ZonedDateTime eventTimestamp;

/**
* Get derived task execution ID.
* <p>ID is derived from instanceId + taskPosition (format: "instanceId:taskPosition")
* <p>No database column - computed from composite key
*/
public String getId() {
return id;
if (id != null) {
return id;
}
if (instanceId != null && taskPosition != null) {
return instanceId + ":" + taskPosition;
}
return null;
Comment on lines +110 to +113
}

/**
* Set task execution ID.
* <p>For deserialization only - ID is normally derived from instanceId + taskPosition
*/
public void setId(String id) {
this.id = id;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,6 @@
}
}
},
"taskExecutionId": {
"type": "text",
"fields": {
"keyword": {
"type": "keyword"
}
}
},
"taskPosition": {
"type": "text",
"fields": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ CREATE INDEX IF NOT EXISTS idx_workflow_instances_start ON workflow_instances (s
CREATE INDEX IF NOT EXISTS idx_workflow_instances_last_event_time ON workflow_instances (last_event_time DESC);

CREATE TABLE IF NOT EXISTS task_instances (
task_execution_id VARCHAR(255),
instance_id VARCHAR(255) NOT NULL,
task_name VARCHAR(255),
task_position VARCHAR(255) NOT NULL,
Expand All @@ -82,16 +81,14 @@ CREATE TABLE IF NOT EXISTS task_instances (
created_at TIMESTAMP WITH TIME ZONE DEFAULT NOW(),
updated_at TIMESTAMP WITH TIME ZONE DEFAULT NOW(),
-- Composite primary key: (instance_id, task_position) uniquely identifies a task
-- Quarkus Flow generates different taskExecutionId per event (started, completed, etc.)
-- Same workflow + same position = same task execution
-- TaskExecution.id is derived from instanceId + taskPosition (no column needed)
PRIMARY KEY (instance_id, task_position),
CONSTRAINT fk_task_instance_workflow FOREIGN KEY (instance_id) REFERENCES workflow_instances(id) ON DELETE CASCADE
);

CREATE INDEX IF NOT EXISTS idx_task_instances_instance_id ON task_instances (instance_id);
CREATE INDEX IF NOT EXISTS idx_task_instances_status ON task_instances (status);
CREATE INDEX IF NOT EXISTS idx_task_instances_last_event_time ON task_instances (last_event_time DESC);
CREATE INDEX IF NOT EXISTS idx_task_instances_task_execution_id ON task_instances (task_execution_id);

-- ============================================================================
-- TRIGGER FUNCTIONS (Extract from JSONB and normalize with idempotency)
Expand Down Expand Up @@ -218,7 +215,6 @@ BEGIN
-- instance already exists because workflow events have already created it.
BEGIN
INSERT INTO task_instances (
task_execution_id,
instance_id,
task_name,
task_position,
Expand All @@ -236,7 +232,6 @@ BEGIN
created_at,
updated_at
) VALUES (
NEW.data->>'taskExecutionId',
NEW.data->>'instanceId',
NEW.data->>'taskName',
NEW.data->>'taskPosition',
Expand Down Expand Up @@ -267,9 +262,6 @@ BEGIN
NEW.time
)
ON CONFLICT (instance_id, task_position) DO UPDATE SET
-- Update task_execution_id to the latest (even though it changes per event)
task_execution_id = EXCLUDED.task_execution_id,

-- Keep first non-null values (immutable fields)
task_name = COALESCE(task_instances.task_name, EXCLUDED.task_name),

Expand Down Expand Up @@ -318,7 +310,6 @@ BEGIN
ON CONFLICT (id) DO NOTHING;

INSERT INTO task_instances (
task_execution_id,
instance_id,
task_name,
task_position,
Expand All @@ -336,7 +327,6 @@ BEGIN
created_at,
updated_at
) VALUES (
NEW.data->>'taskExecutionId',
NEW.data->>'instanceId',
NEW.data->>'taskName',
NEW.data->>'taskPosition',
Expand Down Expand Up @@ -367,9 +357,6 @@ BEGIN
NEW.time
)
ON CONFLICT (instance_id, task_position) DO UPDATE SET
-- Update task_execution_id to the latest (even though it changes per event)
task_execution_id = EXCLUDED.task_execution_id,

-- Keep first non-null values (immutable fields)
task_name = COALESCE(task_instances.task_name, EXCLUDED.task_name),

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,44 @@ protected TaskExecutionJPAStorage() {
super();
}

/**
* Get task execution by derived ID.
* <p>ID format: "instanceId:taskPosition"
* <p>Parses the composite ID and queries by both instanceId and taskPosition.
*
* @param id Derived ID in format "instanceId:taskPosition"
* @return TaskExecution or null if not found
*/
@Override
public TaskExecution get(String id) {
if (id == null || id.isEmpty()) {
return null;
}

// Parse composite ID: "instanceId:taskPosition"
int separatorIndex = id.indexOf(':');
if (separatorIndex == -1 || separatorIndex == 0 || separatorIndex == id.length() - 1) {
throw new IllegalArgumentException("Invalid task execution ID format. Expected 'instanceId:taskPosition', got: " + id);
}
Comment on lines +65 to +83

String instanceId = id.substring(0, separatorIndex);
String taskPosition = id.substring(separatorIndex + 1);

// Query by composite key
List<TaskInstanceEntity> entities = em
.createQuery("SELECT t FROM TaskInstanceEntity t WHERE t.instanceId = :instanceId AND t.taskPosition = :taskPosition", TaskInstanceEntity.class)
.setParameter("instanceId", instanceId)
.setParameter("taskPosition", taskPosition)
.setMaxResults(1)
.getResultList();

if (entities.isEmpty()) {
return null;
}

return mapToModel.apply(entities.get(0));
}

@Override
public List<TaskExecution> findByWorkflowInstanceId(String workflowInstanceId) {
List<TaskInstanceEntity> entities = em
Expand Down
Loading
Loading