twentyhq / twenty

Building a modern alternative to Salesforce, powered by the community.
https://twenty.com
Other
20.25k stars 2.25k forks source link

Select more efficient Sorting Key in Tinybird data sources #8393

Closed gnzjgo closed 1 day ago

gnzjgo commented 5 days ago

Scope & Context

Change Materialized Views Sorting Keys to make API Endpoints more performant. Context here.

Current behavior

Example packages/twenty-tinybird/datasources/webhookEventMV.datasource has the following SK:

ENGINE_SORTING_KEY timestamp, workspaceId

And when consumed in packages/twenty-tinybird/pipes/getWebhookAnalytics.pipe it is always fiktered by webhookId, workspaceId, and timestamp

    FROM webhookEventMV
    WHERE
        true
        AND webhookId = {{ String(webhookId, '90f12aed-0276-4bea-bcaa-c21ea2763d7d', required=True) }}
        AND workspaceId
        ={{ String(workspaceId, '20202020-1c25-4d02-bf25-6aeccf7ea419', required=True) }}
        AND timestamp >= {{ DateTime(start, '2024-10-22 00:00:00') }}
        AND timestamp < {{ DateTime(end, '2024-10-23 00:00:00') }}

Expected behavior

Following best practices:

Some good rules of thumb for setting Sorting Keys:

  • Order matters: Data will be stored based on the Sort Key order.
  • Between 3 and 5 columns is good enough. More will probably penalize.
  • timestamp is often a bad candidate for being the first element of the SK.
  • If you have a multi-tenant app, customer_id is a great candidate for being the first element of the SK.

I'd go for this change:

- ENGINE_SORTING_KEY timestamp, workspaceId
+ ENGINE_SORTING_KEY workspaceId, webhookId, timestamp

PS: same applies for packages/twenty-tinybird/datasources/serverlessFunctionEventMV.datasource

Technical inputs

If using git integration, check this example.
If still prototyping and fine using UI and CLI in the workspace, go for this other one.

FelixMalfait commented 5 days ago

Thanks a lot @gnzjgo!! cc @anamarn I think you might have covered that in your ongoing PR already?

anamarn commented 1 day ago

Yes it's already been covered on PR #8253 :)