fix(data-warehouse): Handle compaction errors

[Gilbert09]

Changes

• If the compaction job errors out, we don't want to fail the external data job workflow too, just carry on as normal
• Increase the timeout for compaction jobs to 1-hour for larger tables

[greptile-apps]

PR Summary

This PR enhances Delta Lake compaction job handling in PostHog's data warehouse system, focusing on error resilience and larger dataset support.

• Added error handling in trigger_compaction_job to catch and log exceptions without failing parent workflows
• Increased compaction job timeout from 5 minutes to 60 minutes to support larger tables
• Added logger parameter to trigger_compaction_job for improved error tracking and debugging
• Modified pipeline to continue execution even if compaction fails, preventing workflow interruption

_{2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile}

[tomasfarias]

Don't we want to re-raise here? Otherwise this will return a workflow-id of a Workflow that failed. But it looks like we are only logging that ID so maybe it's better not to re-raise.

[Gilbert09]

Nah, we want to carry on the external data job as normal - we want to avoid compaction job errors making the external data job fail

[tomasfarias]

Makes sense, thank you!

[tomasfarias]

Changes look okay, but I'm wondering if we don't want the compaction job to be a child workflow (i.e. use start_child_workflow instead of start_workflow).

This would allow cancellations and/or timeouts to propagate to the child (depending on configuration). Currently, the compaction job workflow is independent, meaning it has to be independently cancelled.

Anyways, something for later.

EDIT: I guess this also raises the question of whether we need a child workflow at all, couldn't the compaction job just be another activity?

[Gilbert09]

Changes look okay, but I'm wondering if we don't want the compaction job to be a child workflow (i.e. use start_child_workflow instead of start_workflow).

This would allow cancellations and/or timeouts to propagate to the child (depending on configuration). Currently, the compaction job workflow is independent, meaning it has to be independently cancelled.

Anyways, something for later.

EDIT: I guess this also raises the question of whether we need a child workflow at all, couldn't the compaction job just be another activity?

I guess this also raises the question of whether we need a child workflow at all, couldn't the compaction job just be another activity?

its in a different workflow due to memory issues. Compaction/vacuuming is a lil leaky and can really spike memory. It's a non-critical part of the workflow, and so offloading it onto a another worker (it has its own pods) seemed sensible

Changes look okay, but I'm wondering if we don't want the compaction job to be a child workflow (i.e. use start_child_workflow instead of start_workflow).

Ah, didn't realise a child workflow was a thing. Something for us to look into and see what the advantages of using that are