Skip to content

Conversation

@DanielBadura
Copy link
Member

@DanielBadura DanielBadura commented Jan 6, 2025

We should use jsonb when the user is using postgresql. This give the user better query capabilities on the json fields in the store.

When this PR is released doctrine/dbal#6693 the migration tooling should pick this change up. So the user does not need to update the table themselfes besides of the normal call to bin/console do:mi:di & bin/console do:mi:mi.

@github-actions
Copy link

github-actions bot commented Jan 6, 2025

Hello 👋

here is the most recent benchmark result:

SubscriptionEngineBatchBench
============================

+---------------------------+-------------------+-------------------+-----------+-----------------+------------+-------------+
|                           | time (kde mode)                                   | memory                                     |
+---------------------------+-------------------+-------------------+-----------+-----------------+------------+-------------+
| subject                   | Tag: <current>    | Tag: base         | time-diff | Tag: <current>  | Tag: base  | memory-diff |
+---------------------------+-------------------+-------------------+-----------+-----------------+------------+-------------+
| benchHandle10000Events () | 81.755ms (±0.00%) | 74.500ms (±0.00%) | +9.74%    | 35.340mb        | 35.104mb   | +0.67%      |
+---------------------------+-------------------+-------------------+-----------+-----------------+------------+-------------+

SnapshotsBench
==============

+----------------------------------------+--------------------+--------------------+-----------+-----------------+------------+-------------+
|                                        | time (kde mode)                                     | memory                                     |
+----------------------------------------+--------------------+--------------------+-----------+-----------------+------------+-------------+
| subject                                | Tag: <current>     | Tag: base          | time-diff | Tag: <current>  | Tag: base  | memory-diff |
+----------------------------------------+--------------------+--------------------+-----------+-----------------+------------+-------------+
| benchLoad10000EventsMissingSnapshot () | 57.378ms (±0.00%)  | 51.768ms (±0.00%)  | +10.84%   | 34.418mb        | 34.197mb   | +0.65%      |
| benchLoad10000Events ()                | 935.000μs (±0.00%) | 917.000μs (±0.00%) | +1.96%    | 34.418mb        | 34.197mb   | +0.65%      |
+----------------------------------------+--------------------+--------------------+-----------+-----------------+------------+-------------+

SubscriptionEngineBench
=======================

+---------------------------+-----------------+-----------------+-----------+-----------------+------------+-------------+
|                           | time (kde mode)                               | memory                                     |
+---------------------------+-----------------+-----------------+-----------+-----------------+------------+-------------+
| subject                   | Tag: <current>  | Tag: base       | time-diff | Tag: <current>  | Tag: base  | memory-diff |
+---------------------------+-----------------+-----------------+-----------+-----------------+------------+-------------+
| benchHandle10000Events () | 3.259s (±0.00%) | 3.216s (±0.00%) | +1.34%    | 46.961mb        | 46.740mb   | +0.47%      |
+---------------------------+-----------------+-----------------+-----------+-----------------+------------+-------------+

SimpleSetupStreamStoreBench
===========================

+----------------------------------------+--------------------+--------------------+-----------+-----------------+------------+-------------+
|                                        | time (kde mode)                                     | memory                                     |
+----------------------------------------+--------------------+--------------------+-----------+-----------------+------------+-------------+
| subject                                | Tag: <current>     | Tag: base          | time-diff | Tag: <current>  | Tag: base  | memory-diff |
+----------------------------------------+--------------------+--------------------+-----------+-----------------+------------+-------------+
| benchLoad1Event ()                     | 1.045ms (±0.00%)   | 1.003ms (±0.00%)   | +4.23%    | 34.458mb        | 34.306mb   | +0.44%      |
| benchLoad10000Events ()                | 63.263ms (±0.00%)  | 56.492ms (±0.00%)  | +11.99%   | 34.458mb        | 34.306mb   | +0.44%      |
| benchSave1Event ()                     | 1.006ms (±0.00%)   | 1.069ms (±0.00%)   | -5.91%    | 34.458mb        | 34.306mb   | +0.44%      |
| benchSave10000Events ()                | 341.747ms (±0.00%) | 295.775ms (±0.00%) | +15.54%   | 34.458mb        | 34.306mb   | +0.44%      |
| benchSave10000Aggregates ()            | 8.203s (±0.00%)    | 8.080s (±0.00%)    | +1.51%    | 34.458mb        | 34.306mb   | +0.44%      |
| benchSave10000AggregatesTransaction () | 5.163s (±0.00%)    | 5.136s (±0.00%)    | +0.51%    | 34.458mb        | 34.306mb   | +0.44%      |
+----------------------------------------+--------------------+--------------------+-----------+-----------------+------------+-------------+

SplitStreamBench
================

+-------------------------+--------------------+--------------------+-----------+-----------------+------------+-------------+
|                         | time (kde mode)                                     | memory                                     |
+-------------------------+--------------------+--------------------+-----------+-----------------+------------+-------------+
| subject                 | Tag: <current>     | Tag: base          | time-diff | Tag: <current>  | Tag: base  | memory-diff |
+-------------------------+--------------------+--------------------+-----------+-----------------+------------+-------------+
| benchLoad10000Events () | 4.463ms (±0.00%)   | 4.406ms (±0.00%)   | +1.29%    | 37.651mb        | 37.499mb   | +0.41%      |
| benchSave10000Events () | 367.048ms (±0.00%) | 337.239ms (±0.00%) | +8.84%    | 37.723mb        | 37.501mb   | +0.59%      |
+-------------------------+--------------------+--------------------+-----------+-----------------+------------+-------------+

SimpleSetupBench
================

+----------------------------------------+--------------------+--------------------+-----------+-----------------+------------+-------------+
|                                        | time (kde mode)                                     | memory                                     |
+----------------------------------------+--------------------+--------------------+-----------+-----------------+------------+-------------+
| subject                                | Tag: <current>     | Tag: base          | time-diff | Tag: <current>  | Tag: base  | memory-diff |
+----------------------------------------+--------------------+--------------------+-----------+-----------------+------------+-------------+
| benchLoad1Event ()                     | 994.800μs (±0.00%) | 957.200μs (±0.00%) | +3.93%    | 34.344mb        | 34.123mb   | +0.65%      |
| benchLoad10000Events ()                | 56.707ms (±0.00%)  | 52.344ms (±0.00%)  | +8.34%    | 34.344mb        | 34.123mb   | +0.65%      |
| benchSave1Event ()                     | 1.041ms (±0.00%)   | 1.004ms (±0.00%)   | +3.70%    | 34.344mb        | 34.123mb   | +0.65%      |
| benchSave10000Events ()                | 245.760ms (±0.00%) | 216.501ms (±0.00%) | +13.51%   | 34.344mb        | 34.123mb   | +0.65%      |
| benchSave10000Aggregates ()            | 7.871s (±0.00%)    | 7.835s (±0.00%)    | +0.46%    | 34.344mb        | 34.123mb   | +0.65%      |
| benchSave10000AggregatesTransaction () | 4.985s (±0.00%)    | 4.952s (±0.00%)    | +0.68%    | 34.344mb        | 34.123mb   | +0.65%      |
+----------------------------------------+--------------------+--------------------+-----------+-----------------+------------+-------------+

PersonalDataBench
=================

+----------------------------------------+--------------------+--------------------+-----------+-----------------+------------+-------------+
|                                        | time (kde mode)                                     | memory                                     |
+----------------------------------------+--------------------+--------------------+-----------+-----------------+------------+-------------+
| subject                                | Tag: <current>     | Tag: base          | time-diff | Tag: <current>  | Tag: base  | memory-diff |
+----------------------------------------+--------------------+--------------------+-----------+-----------------+------------+-------------+
| benchLoad1Event ()                     | 962.400μs (±0.00%) | 891.400μs (±0.00%) | +7.96%    | 35.548mb        | 35.252mb   | +0.84%      |
| benchLoad10000Events ()                | 105.875ms (±0.00%) | 89.178ms (±0.00%)  | +18.72%   | 35.548mb        | 35.253mb   | +0.84%      |
| benchSave1Event ()                     | 1.445ms (±0.00%)   | 1.488ms (±0.00%)   | -2.91%    | 35.548mb        | 35.252mb   | +0.84%      |
| benchSave10000Events ()                | 293.307ms (±0.00%) | 253.221ms (±0.00%) | +15.83%   | 35.550mb        | 35.254mb   | +0.84%      |
| benchSave10000Aggregates ()            | 12.140s (±0.00%)   | 12.075s (±0.00%)   | +0.53%    | 35.548mb        | 35.253mb   | +0.84%      |
| benchSave10000AggregatesTransaction () | 9.184s (±0.00%)    | 9.183s (±0.00%)    | +0.02%    | 36.050mb        | 35.753mb   | +0.83%      |
+----------------------------------------+--------------------+--------------------+-----------+-----------------+------------+-------------+

This comment gets update everytime a new commit comes in!

private static function transformTrace(array $trace): array
{
if (array_key_exists('class', $trace) && is_string($trace['class'])) {
$trace['class'] = str_replace("\x00", '', $trace['class']);
Copy link
Member Author

Choose a reason for hiding this comment

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

Any better suggestions @DavidBadura ?

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't that be its own PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right now, this is not doing any harm. When we adjust the table type to jsonb Postgres checks if the json is valid and doesn't like null bytes in there.

I don't think that this must be in a separate bugfix PR but I could extract it. The only thing is if the user already has null bytes in the table. Then he would need to remove them. Or do you have a case in mind?

@DanielBadura
Copy link
Member Author

New version for dbal was made which makes this change easier for our users.

Still need to evaluate if this is worth it due to slower write speed compared to the only limited gain for querying the eventstore.

@DavidBadura DavidBadura added this to the 4.0.0 milestone Jun 11, 2025
@DanielBadura DanielBadura changed the base branch from 3.8.x to 3.12.x June 18, 2025 12:29
@DanielBadura
Copy link
Member Author

DanielBadura commented Jun 18, 2025

I rebased this PR and would like to investigate into this again in the next days/weeks.
Also, i think that the changes for the subscription table are probably not really needed and therefore the null byte removal is also not needed. Why? Because this is a real edge case to query the subscription table manually with advanced json features on the stack trace.

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.

2 participants