Skip to content

Conversation

@maxdml
Copy link
Collaborator

@maxdml maxdml commented Jul 24, 2025

  • Implement set/get events
  • Modify the notification system to use condition variables: while send/recv work fine with 1 producer: 1 consumer, set/get events allow for concurrent consumers.
  • Add a log entry when dbos.Launch() recover workflows.
  • Fix transaction usage in Recv()

Comment on lines +1179 to +1183
tx, err := s.pool.Begin(ctx)
if err != nil {
return nil, fmt.Errorf("failed to begin transaction: %w", err)
}
defer tx.Rollback(ctx)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The transaction should start for the sequence:

  • consume message
  • record Recv() output

But not before


return value, nil
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I trust recording the step outcome + reading the event do not need being done transactionally.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, they don't, as it doesn't consume the event

Comment on lines -1088 to -1129

// Send and receive again the same workflows to verify idempotency
_, err = sendWf(context.Background(), sendWorkflowInput{
DestinationID: receiveHandle.GetWorkflowID(),
Topic: "test-topic",
}, WithWorkflowID(handle.GetWorkflowID()))
if err != nil {
t.Fatalf("failed to send message with same workflow ID: %v", err)
}
receiveHandle2, err := receiveWf(context.Background(), "test-topic", WithWorkflowID(receiveHandle.GetWorkflowID()))
if err != nil {
t.Fatalf("failed to start receive workflow with same ID: %v", err)
}
result, err = receiveHandle2.GetResult(context.Background())
if err != nil {
t.Fatalf("failed to get result from receive workflow with same ID: %v", err)
}
if result != "message1-message2-message3" {
t.Fatalf("expected received message to be 'message1-message2-message3', got '%s'", result)
}

// Get steps for both workflows and verify we have the expected number
sendSteps, err := dbos.systemDB.GetWorkflowSteps(context.Background(), handle.GetWorkflowID())
if err != nil {
t.Fatalf("failed to get steps for send workflow: %v", err)
}
receiveSteps, err := dbos.systemDB.GetWorkflowSteps(context.Background(), receiveHandle.GetWorkflowID())
if err != nil {
t.Fatalf("failed to get steps for receive workflow: %v", err)
}

// Verify the number of steps matches the number of send() and recv() calls
// sendWorkflow has 3 Send() calls, receiveWorkflow has 3 Recv() calls
expectedSendSteps := 3
expectedReceiveSteps := 3

if len(sendSteps) != expectedSendSteps {
t.Fatalf("expected %d steps in send workflow, got %d", expectedSendSteps, len(sendSteps))
}
if len(receiveSteps) != expectedReceiveSteps {
t.Fatalf("expected %d steps in receive workflow, got %d", expectedReceiveSteps, len(receiveSteps))
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was testing the workflow idempotency, not the steps -- already tested elsewhere.

@maxdml maxdml merged commit 1d134da into main Jul 24, 2025
1 check passed
@maxdml maxdml deleted the send-get-events branch July 24, 2025 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants