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

Support incrementally updating group properties #7154

Closed
macobo opened this issue Nov 16, 2021 · 4 comments
Closed

Support incrementally updating group properties #7154

macobo opened this issue Nov 16, 2021 · 4 comments
Labels
enhancement New feature or request feature/group-analytics Feeature Tag: Group analytics feature/ingestion

Comments

@macobo
Copy link
Contributor

macobo commented Nov 16, 2021

Is your feature request related to a problem?

Currently, group properties can only be updated all at once.

Additionally, the created_at column on groups is a lie (it's actually last_updated)

Describe the solution you'd like

Use the same new pattern team platform has settled on for updating person properties. Likely this will include some form of:

  • Storing the group properties in postgres as well
  • Doing versioning.

Exact details TBD.

Describe alternatives you've considered

Not supporting incremental updates. This works for now, but see below for a reason to do it.

Additional context

This has become blocker for #7010 as well as querying clickhouse in /decide endpoint is not really ideal.

Thank you for your feature request – we love each and every one!

@macobo macobo added enhancement New feature or request people feature/ingestion feature/group-analytics Feeature Tag: Group analytics labels Nov 16, 2021
@yakkomajuri
Copy link
Contributor

Dump from Slack:

#6834 is the place with the most context.
But the TL;DR is:
Me and Tiina laid out how a property should be updated based on the following variables: the event timestamp, the last timestamp of a property update, the current operation (set/set_once), and the previous operation (set/set_once).
Based on the above, you should lock a groups row (with SELECT FOR UPDATE), calculate the property updates, and issue the update.
A lot of the logic for checking is already in place.

You’d need a Postgres table with at least properties, properties_last_updated_at , and properties_last_operation.

@yakkomajuri
Copy link
Contributor

I also would recommend the follow-up to this being the equivalent version work #6667

@macobo
Copy link
Contributor Author

macobo commented Nov 16, 2021

Chatted with @yakkomajuri how to go about this. Notes from that:

  • We'll go forward with this - person updates in particular are now in and ready to be followed as a pattern.
  • Important columns for the postgres table are properties, properties_last_operation, properties_last_updated_at all jsonb and version, which is populated for pg.
  • During updates we should lock relevant rows (SELECT FOR UPDATE)
  • For clickhouse version is not yet ready and I won't be building for it now. Doing this depends on either EPIC: Handling big migrations for self-hosted users #7054 or doing it in a slightly smarter way. Will follow up with this with #team-platform later

@macobo
Copy link
Contributor Author

macobo commented Nov 18, 2021

Initial support for this has landed!

@macobo macobo closed this as completed Nov 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature/group-analytics Feeature Tag: Group analytics feature/ingestion
Projects
None yet
Development

No branches or pull requests

2 participants