Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(filemanager): sequencer ordering #396

Merged
merged 2 commits into from
Jul 8, 2024
Merged

Conversation

mmalenic
Copy link
Member

@mmalenic mmalenic commented Jul 8, 2024

Closes #395

Changes

  • Order by sequencer in list API queries.
  • Add an index on the sequencer to support better sorting of ingested events.
    • I think this effectively does what this piece of code is doing, except it's a native construct to the database:
      -- And filter them to the objects that need to be updated.
      objects_to_update as (
      select
      *
      from current_objects
      where
      -- Check the sequencer condition. We only update if there is a created
      -- sequencer that is closer to the deleted sequencer.
      current_objects.deleted_sequencer > current_objects.input_created_sequencer and
      (
      -- Updating a null sequencer doesn't cause the event to be reprocessed.
      current_objects.sequencer is null or
      -- If a sequencer already exists this event should be reprocessed because this
      -- sequencer could belong to another object.
      current_objects.sequencer < current_objects.input_created_sequencer
      ) and
      -- And there should not be any objects with a created sequencer that is the same as the input created
      -- sequencer because this is a duplicate event that would cause a constraint error in the update.
      current_objects.input_created_sequencer not in (
      select sequencer from current_objects where sequencer is not null
      )
      -- Only one event entry should be updated, and that entry must be the one with the
      -- deleted sequencer that is minimum, i.e. closest to the created sequencer which
      -- is going to be inserted.
      order by current_objects.deleted_sequencer asc
      limit 1
      ),
      This is one of the reasons why I simplified the database ingestion, because I think a similar trade off with ingest/query performance can be achieved by just using an index (although I would need to test this to be sure).

@mmalenic mmalenic self-assigned this Jul 8, 2024
@mmalenic mmalenic added fix filemanager an issue relating to the filemanager labels Jul 8, 2024
@mmalenic mmalenic requested review from brainstorm and victorskl July 8, 2024 03:05
@mmalenic mmalenic changed the title Fix/sequencer ordering fix(filemanager): sequencer ordering Jul 8, 2024
Copy link
Member

@brainstorm brainstorm left a comment

Choose a reason for hiding this comment

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

🚀

@mmalenic mmalenic merged commit 6b990f7 into main Jul 8, 2024
5 checks passed
@mmalenic mmalenic deleted the fix/sequencer-ordering branch July 8, 2024 05:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
filemanager an issue relating to the filemanager fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

filemanager: order by sequencer
3 participants