Skip to content

Denormalize event type for request and event logs#829

Closed
ezekg wants to merge 3 commits intomasterfrom
feature/denormalize-event-type-for-request-logs
Closed

Denormalize event type for request and event logs#829
ezekg wants to merge 3 commits intomasterfrom
feature/denormalize-event-type-for-request-logs

Conversation

@ezekg
Copy link
Member

@ezekg ezekg commented May 1, 2024

Part 1 of 4. Improves performance for Dashboard metrics.

Prerequisites

  • Cut new Keygen CE/EE versions so we release multi-user licenses before this.

Post deploy

Part 2 (in order of operations)

  • Backfill denormalized event_type_event from event_types to event_logs.
  • Add NOT NULL constraint to event_logs.event_type_event.
  • Add index on event_logs.event_type_event.
  • Run VACUUM ANALYZE event_logs.

Part 3 (in order of operations)

  • Backfill denormalized event_type_id and event_type_event from event_logs to request_logs.
  • Add NOT NULL constraints to request_logs.event_type_id and request_logs.event_type_event.
  • Add indexes on request_logs.event_type_id and request_logs.event_type_event.
  • Run VACUUM ANALYZE request_logs.

Part 4

  • Update EventLog.for_event_type to use denormalized event_type_event column for strings.
  • Update RequestLog.for_event_type to use denormalized event_type_event column.

@ezekg ezekg force-pushed the feature/denormalize-event-type-for-request-logs branch from 25135c0 to d0ebf37 Compare May 1, 2024 13:54
@ezekg ezekg force-pushed the feature/denormalize-event-type-for-request-logs branch from d0ebf37 to d78f333 Compare May 1, 2024 14:06
@ezekg ezekg force-pushed the feature/denormalize-event-type-for-request-logs branch from d78f333 to 5dde815 Compare May 1, 2024 14:50
@ezekg ezekg marked this pull request as ready for review May 1, 2024 15:59
has_environment
has_account

denormalizes :event, from: :event_type, prefix: :event_type
Copy link
Member Author

@ezekg ezekg Jan 13, 2025

Choose a reason for hiding this comment

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

Should we be denormalizing event_type_id from event_log too? Would solve for the race condition touched on here.

has_account

denormalizes :event, from: :event_type, prefix: :event_type
denormalizes :event_type_id, to: :request_log
Copy link
Member Author

@ezekg ezekg Jan 13, 2025

Choose a reason for hiding this comment

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

Possible race condition here since there's no guarantee an event log will be inserted after a request log. Need to rethink this (and write appropriate tests).


belongs_to :requestor, polymorphic: true, optional: true
belongs_to :resource, polymorphic: true, optional: true
belongs_to :event_type, optional: true
Copy link
Member Author

Choose a reason for hiding this comment

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

Where is this being set?

@ezekg
Copy link
Member Author

ezekg commented Apr 3, 2025

Closing because this is outdated and denormalizing a string column onto every event log row is going to be super expensive, storage-wise. I'd rather look into using materialized views or something to improve performance here.

@ezekg ezekg closed this Apr 3, 2025
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