Repository navigation
Remove redundant task_execution_id column (#31) - #35
Closed
ricardozanini wants to merge 3 commits into
Closed
ricardozanini wants to merge 3 commits into
ricardozanini wants to merge 3 commits into
Conversation
Design for 3 new slides (16-18) to be added to Data Index POC presentation: - Slide 16: MODE 3 Architecture (Kafka + Direct Write) - Slide 17: Three-way comparison (MODE 1 vs MODE 2 vs MODE 3) - Slide 18: Updated decision framework Sequential evolution narrative: FluentBit → Elasticsearch → Kafka Focus on architecture and decision criteria, no implementation timeline Related to #23
Plan includes: - Part 1: Add 3 MODE 3 slides to presentation (16-18) - Part 2: Deploy MODE 1 to KIND and run e2e tests - 15 tasks with bite-sized steps - Quick reference guide for presentation demo Related to #23
Remove task_execution_id column from task_instances table across all modes. TaskExecution.id is now derived from instanceId + taskPosition composite key. Database (MODE 1 - PostgreSQL): - Update V1__initial_schema.sql to remove task_execution_id column - Update trigger function to remove task_execution_id from INSERT/UPDATE - Primary key remains (instance_id, task_position) Domain Model: - TaskExecution.getId() now derives ID from instanceId + taskPosition - Format: "instanceId:taskPosition" JPA (MODE 1): - TaskInstanceEntity uses composite @id on instanceId and taskPosition - Update equals() and hashCode() to use composite key - TaskInstanceEntityMapper updated for derived ID - TaskExecutionJPAStorage.get() parses derived ID and queries by composite key Kafka Ingestion (MODE 3): - Remove generateTaskExecutionId() from Mapper and TaskExecutionProcessor - Update task-instance-upsert.sql to remove task_execution_id column - Update TaskPersistence.setTaskParameters() for new column count Elasticsearch (MODE 2): - Remove taskExecutionId from task-events.json index template GraphQL API: - TaskExecutionJPAStorage.get() parses "instanceId:taskPosition" format - Queries by composite key (instance_id, task_position) Tests: - Remove setTaskExecutionId() calls from WorkflowInstanceGraphQLApiTest - ID now derived automatically from instanceId + taskPosition Documentation: - Update CLAUDE.md field mapping table - Update composite key section to reflect column removal - Add guidance on derived ID format Fixes #31
Contributor
Author
|
Closing - tests failing with composite ID implementation. Need to revise approach. |
There was a problem hiding this comment.
Pull request overview
This PR removes the redundant task_execution_id column from the task_instances normalized table and standardizes task identity on the composite key (instance_id, task_position), with TaskExecution.id derived as instanceId:taskPosition. This aligns MODE 1 (PostgreSQL triggers), MODE 2 (Elasticsearch raw event mapping), and MODE 3 (Kafka ingestion) around a single, stable identity strategy.
Changes:
- Removed
task_execution_idfrom the MODE 1 V1 schema and trigger logic, keeping(instance_id, task_position)as the PK. - Updated the domain model and JPA layer to derive/parse task IDs in the format
instanceId:taskPosition. - Removed
taskExecutionIdfrom the Elasticsearchtask-eventsindex template and adjusted MODE 3 Kafka ingestion SQL/parameter mapping accordingly.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/superpowers/specs/2026-05-22-mode3-kafka-presentation-design.md | Adds MODE 3 presentation slide design doc (appears unrelated to task_execution_id removal scope). |
| docs/superpowers/plans/2026-05-22-mode3-presentation-e2e-tests.md | Adds a detailed presentation + KIND e2e execution plan (appears unrelated to this PR’s stated scope). |
| data-index/data-index-storage/data-index-storage-postgresql/src/main/java/org/kubesmarts/logic/dataindex/storage/jpa/TaskExecutionJPAStorage.java | Implements derived-ID parsing (instanceId:taskPosition) for lookups. |
| data-index/data-index-storage/data-index-storage-postgresql/src/main/java/org/kubesmarts/logic/dataindex/storage/jpa/mapper/TaskInstanceEntityMapper.java | Updates entity↔model mapping to align with derived ID and composite key fields. |
| data-index/data-index-storage/data-index-storage-postgresql/src/main/java/org/kubesmarts/logic/dataindex/storage/jpa/entity/TaskInstanceEntity.java | Removes taskExecutionId field; models identity via instanceId + taskPosition and derives getId(). |
| data-index/data-index-storage/data-index-storage-migrations/src/main/resources/db/migration/V1__initial_schema.sql | Drops task_execution_id column/index and removes trigger references. |
| data-index/data-index-storage/data-index-storage-elasticsearch-schema/src/main/resources/elasticsearch/index-templates/task-events.json | Removes taskExecutionId mapping from raw task events template. |
| data-index/data-index-model/src/main/java/org/kubesmarts/logic/dataindex/model/TaskExecution.java | Derives id from (instanceId, taskPosition) when not explicitly set. |
| data-index/data-index-integration-tests/data-index-integration-tests-postgresql/src/test/java/org/kubesmarts/logic/dataindex/graphql/WorkflowInstanceGraphQLApiTest.java | Removes now-nonexistent setTaskExecutionId() usage in test data setup. |
| data-index/data-index-ingestion/data-index-ingestion-kafka-service/src/main/java/org/kubesmarts/logic/dataindex/ingestion/kafka/service/Mapper.java | Stops generating task IDs in the Kafka service mapper. |
| data-index/data-index-ingestion/data-index-ingestion-kafka-processor/src/main/resources/sql/task-instance-upsert.sql | Removes task_execution_id from MODE 3 upsert statement. |
| data-index/data-index-ingestion/data-index-ingestion-kafka-processor/src/main/java/org/kubesmarts/logic/dataindex/ingestion/kafka/processor/TaskExecutionProcessor.java | Stops setting task ID during batch processing. |
| data-index/data-index-ingestion/data-index-ingestion-kafka-processor/src/main/java/org/kubesmarts/logic/dataindex/ingestion/kafka/processor/persistence/TaskPersistence.java | Adjusts JDBC parameter binding order/count to match SQL column removal. |
| CLAUDE.md | Updates internal architecture/docs guidance for derived task ID and composite PK approach. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+110
to
+113
| if (instanceId != null && taskPosition != null) { | ||
| return instanceId + ":" + taskPosition; | ||
| } | ||
| return null; |
Comment on lines
55
to
57
| @Entity | ||
| @Table(name = "task_instances") | ||
| public class TaskInstanceEntity extends AbstractEntity { |
Comment on lines
26
to
35
| "instanceId": { | ||
| "type": "text", | ||
| "fields": { | ||
| "keyword": { | ||
| "type": "keyword" | ||
| } | ||
| } | ||
| }, | ||
| "taskExecutionId": { | ||
| "type": "text", | ||
| "fields": { | ||
| "keyword": { | ||
| "type": "keyword" | ||
| } | ||
| } | ||
| }, | ||
| "taskPosition": { | ||
| "type": "text", |
Comment on lines
+1
to
+20
| # MODE 3 Kafka Presentation Slides Design | ||
|
|
||
| **Date:** 2026-05-22 | ||
| **Purpose:** Add MODE 3 (Kafka-based ingestion) slides to Data Index POC presentation | ||
| **Scope:** 3 new slides (16-18) for architecture comparison and decision framework | ||
| **Target Audience:** Backend engineers + technical leadership | ||
| **Presentation Date:** 2026-05-22 | ||
|
|
||
| --- | ||
|
|
||
| ## Overview | ||
|
|
||
| This design documents the addition of 3 slides to the existing Data Index v1.0.0 POC presentation to introduce MODE 3 (Kafka-based event ingestion for PostgreSQL). The slides present MODE 3 as a sequential evolution from MODE 1 and MODE 2, providing architectural comparison and decision criteria without committing to an implementation timeline. | ||
|
|
||
| ### Context | ||
|
|
||
| - **Existing presentation:** 15 slides covering MODE 1 (PostgreSQL + Triggers) and MODE 2 (Elasticsearch + Transforms) | ||
| - **GitHub Issue:** #23 - Implement Kafka-based event ingestion service for PostgreSQL backend (MODE 3) | ||
| - **Status:** MODE 3 is designed but not yet implemented | ||
| - **Goal:** Present MODE 3 as a viable architectural option alongside MODE 1 and MODE 2 |
Comment on lines
+65
to
+83
| /** | ||
| * 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
+412
to
+415
| **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) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Removes the redundant
task_execution_idcolumn from thetask_instancestable across all deployment modes. The task execution ID is now derived from the composite key(instance_id, task_position).Motivation
The
task_execution_idcolumn was redundant since the composite primary key(instance_id, task_position)uniquely identifies each task execution. In MODE 3 (Kafka), the ID was already being derived from these two fields, confirming that the column adds no additional value.Removing it:
Changes
Database (MODE 1 - PostgreSQL)
V1__initial_schema.sqlto removetask_execution_idcolumnnormalize_task_event()trigger to removetask_execution_idreferences(instance_id, task_position)Domain Model
TaskExecution.getId()derives ID frominstanceId + ":" + taskPositionJPA Storage (MODE 1)
TaskInstanceEntityuses composite@IdoninstanceIdandtaskPositionequals()andhashCode()to use composite keyTaskInstanceEntityMapperupdated to handle derived IDTaskExecutionJPAStorage.get()parses derived ID and queries by composite keyKafka Ingestion (MODE 3)
generateTaskExecutionId()fromMapperandTaskExecutionProcessortask-instance-upsert.sqlto removetask_execution_idcolumnTaskPersistence.setTaskParameters()for new parameter countElasticsearch (MODE 2)
taskExecutionIdfromtask-events.jsonindex templateGraphQL API
TaskExecutionJPAStorage.get(id)parses ID format"instanceId:taskPosition"Tests
setTaskExecutionId()calls from integration testsinstanceId+taskPositionDocumentation
Test Plan
mvn test -Dquarkus.profile=postgresqlBreaking Changes
None - this is a breaking change to the database schema, but since there's no release yet (v1.0.0 in development), we're updating V1 migration directly rather than creating a V2.
Closes #31