perf(eventstore): optimize commands to events function - #9092
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Thanks for your contribution @muhlemmer! 🎉Please make sure you tick the following checkboxes before marking this Pull Request (PR) as ready for review:
|
This comment was marked as outdated.
This comment was marked as outdated.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9092 +/- ##
=======================================
Coverage ? 62.27%
=======================================
Files ? 1571
Lines ? 147879
Branches ? 0
=======================================
Hits ? 92094
Misses ? 51268
Partials ? 4517
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
I propose to use PLPGSQL functions for postgres, this will not work for cockroachdb free users so i would use an sql function there, we might can benefit from The PLPGSQL function i got best results so far is the following: CREATE OR REPLACE FUNCTION eventstore.latest_aggregate_state(
instance_id TEXT
, aggregate_type TEXT
, aggregate_id TEXT
, sequence OUT BIGINT
, owner OUT TEXT
)
LANGUAGE 'plpgsql'
STABLE PARALLEL SAFE
AS $BODY$
BEGIN
SELECT
COALESCE(e.sequence, 0) AS sequence
, e.owner
INTO
sequence
, owner
FROM
eventstore.events2 e
WHERE
e.instance_id = $1
AND e.aggregate_type = $2
AND e.aggregate_id = $3
ORDER BY
e.sequence DESC
LIMIT 1;
RETURN;
END;
$BODY$;
CREATE OR REPLACE FUNCTION eventstore.commands_to_events(commands eventstore.command[])
RETURNS SETOF eventstore.events2
LANGUAGE 'plpgsql'
STABLE PARALLEL SAFE
ROWS 10
AS $BODY$
DECLARE
"aggregate" RECORD;
current_sequence BIGINT;
current_owner TEXT;
BEGIN
FOR "aggregate" IN
SELECT DISTINCT
instance_id
, aggregate_type
, aggregate_id
FROM UNNEST(commands)
LOOP
SELECT
*
INTO
current_sequence
, current_owner
FROM eventstore.latest_aggregate_state(
"aggregate".instance_id
, "aggregate".aggregate_type
, "aggregate".aggregate_id
);
RETURN QUERY
SELECT
c.instance_id
, c.aggregate_type
, c.aggregate_id
, c.command_type -- AS event_type
, COALESCE(current_sequence, 0) + ROW_NUMBER() -- AS sequence
, c.revision
, NOW() -- AS created_at
, c.payload
, c.creator
, COALESCE(current_owner, c.owner) -- AS owner
, EXTRACT(EPOCH FROM NOW()) -- AS position
, c.ordinality::INT -- AS in_tx_order
FROM
UNNEST(commands) WITH ORDINALITY AS c
WHERE
c.instance_id = aggregate.instance_id
AND c.aggregate_type = aggregate.aggregate_type
AND c.aggregate_id = aggregate.aggregate_id;
END LOOP;
RETURN;
END;
$BODY$; |
|
This PR is as done as it gets. However previously discussed with @adlerhurst:
We didn't think about the fact CTEs are not yet supported by Cockroach either. So Cockroach can't be modified in the same way. As it appears we need to maintain 2 versions of the What is your opinion, @adlerhurst? |
Sounds like a good idea. I created an issue to track if crdb is compatible: #9131 |
|
@adlerhurst I've modified the PR to use the proposed |
Pull request was converted to draft
This comment was marked as outdated.
This comment was marked as outdated.
# Which Problems Are Solved We were seeing high query costs in a the lateral join executed in the commands_to_events procedural function in the database. The high cost resulted in incremental CPU usage as a load test continued and less req/sec handled, sarting at 836 and ending at 130 req/sec. # How the Problems Are Solved 1. Set `PARALLEL SAFE`. I noticed that this option defaults to `UNSAFE`. But it's actually safe if the function doesn't `INSERT` 2. Set the returned `ROWS 10` parameter. 3. Function is re-written in Pl/PgSQL so that we eliminate expensive joins. 4. Introduced an intermediate state that does `SELECT DISTINCT` for the aggregate so that we don't have to do an expensive lateral join. # Additional Changes Use a `COALESCE` to get the owner from the last event, instead of a `CASE` switch. # Additional Context - Function was introduced in #8816 - Closes #8352 --------- Co-authored-by: Silvan <27845747+adlerhurst@users.noreply.github.com>
# Which Problems Are Solved We were seeing high query costs in a the lateral join executed in the commands_to_events procedural function in the database. The high cost resulted in incremental CPU usage as a load test continued and less req/sec handled, sarting at 836 and ending at 130 req/sec. # How the Problems Are Solved 1. Set `PARALLEL SAFE`. I noticed that this option defaults to `UNSAFE`. But it's actually safe if the function doesn't `INSERT` 2. Set the returned `ROWS 10` parameter. 3. Function is re-written in Pl/PgSQL so that we eliminate expensive joins. 4. Introduced an intermediate state that does `SELECT DISTINCT` for the aggregate so that we don't have to do an expensive lateral join. # Additional Changes Use a `COALESCE` to get the owner from the last event, instead of a `CASE` switch. # Additional Context - Function was introduced in #8816 - Closes #8352 --------- Co-authored-by: Silvan <27845747+adlerhurst@users.noreply.github.com>
# Which Problems Are Solved The performance of the initial push function can further be increased # How the Problems Are Solved `eventstore.push`- and `eventstore.commands_to_events`-functions were rewritten # Additional Changes none # Additional Context same optimizations as for postgres: #9092
# Which Problems Are Solved The performance of the initial push function can further be increased # How the Problems Are Solved `eventstore.push`- and `eventstore.commands_to_events`-functions were rewritten # Additional Changes none # Additional Context same optimizations as for postgres: #9092 (cherry picked from commit 690147b)
# Which Problems Are Solved The performance of the initial push function can further be increased # How the Problems Are Solved `eventstore.push`- and `eventstore.commands_to_events`-functions were rewritten # Additional Changes none # Additional Context same optimizations as for postgres: #9092 (cherry picked from commit 690147b)
# Which Problems Are Solved We were seeing high query costs in a the lateral join executed in the commands_to_events procedural function in the database. The high cost resulted in incremental CPU usage as a load test continued and less req/sec handled, sarting at 836 and ending at 130 req/sec. # How the Problems Are Solved 1. Set `PARALLEL SAFE`. I noticed that this option defaults to `UNSAFE`. But it's actually safe if the function doesn't `INSERT` 2. Set the returned `ROWS 10` parameter. 3. Function is re-written in Pl/PgSQL so that we eliminate expensive joins. 4. Introduced an intermediate state that does `SELECT DISTINCT` for the aggregate so that we don't have to do an expensive lateral join. # Additional Changes Use a `COALESCE` to get the owner from the last event, instead of a `CASE` switch. # Additional Context - Function was introduced in zitadel#8816 - Closes zitadel#8352 --------- Co-authored-by: Silvan <27845747+adlerhurst@users.noreply.github.com>
# Which Problems Are Solved The performance of the initial push function can further be increased # How the Problems Are Solved `eventstore.push`- and `eventstore.commands_to_events`-functions were rewritten # Additional Changes none # Additional Context same optimizations as for postgres: zitadel#9092 (cherry picked from commit f2a96e8)
Which Problems Are Solved
We were seeing high query costs in a the lateral join executed in the
commands_to_events procedural function in the database. The high cost
resulted in incremental CPU usage as a load test continued and less
req/sec handled, sarting at 836 and ending at 130 req/sec.
How the Problems Are Solved
PARALLEL SAFE. I noticed that this option defaults toUNSAFE.But it's actually safe if the function doesn't
INSERTROWS 10parameter.SELECT DISTINCTfor theaggregate so that we don't have to do an expensive lateral join.
Additional Changes
Use a
COALESCEto get the owner from the last event, instead of aCASEswitch.Additional Context
refactor(eventstore): move push logic to sql #8816