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

Event property counter #7500

Merged
merged 56 commits into from
Dec 17, 2021
Merged
Changes from 1 commit
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
598c213
create event property model
mariusandra Dec 3, 2021
b609261
add null
mariusandra Dec 3, 2021
01307be
rename cache vars
mariusandra Dec 3, 2021
37aad8c
update event properties table on ingestion
mariusandra Dec 3, 2021
ed359ea
match date formats
mariusandra Dec 3, 2021
8f80f87
match date formats
mariusandra Dec 3, 2021
17cb696
better string handling
mariusandra Dec 3, 2021
304a740
property type can be null too
mariusandra Dec 3, 2021
586c39b
pass event timestamp
mariusandra Dec 3, 2021
d738f9b
update property type later
mariusandra Dec 3, 2021
4cbc1cc
perform all updates through a buffer object
mariusandra Dec 3, 2021
1ad5949
move to EventPropertyCounter
mariusandra Dec 3, 2021
b9be65f
Merge branch 'master' into event-property-counter
mariusandra Dec 14, 2021
2227052
fix migration
mariusandra Dec 14, 2021
4a1da7c
improve flush last seen at job
mariusandra Dec 14, 2021
e64062a
flush job periodically + env
mariusandra Dec 14, 2021
129532b
upsert all event properties in 1 query
mariusandra Dec 14, 2021
ba1b831
log to statsd
mariusandra Dec 14, 2021
2c3c27d
enable property counter only if experimental mode enabled
mariusandra Dec 14, 2021
f69e68c
use now() instead of event timestamp
mariusandra Dec 14, 2021
1c2a777
fix seconds
mariusandra Dec 14, 2021
356d341
Merge branch 'master' into event-property-counter
mariusandra Dec 14, 2021
6215919
add user/pass for default postgres
mariusandra Dec 14, 2021
30ddc74
add tests
mariusandra Dec 14, 2021
48842e1
use big integers
mariusandra Dec 14, 2021
8e7429f
make query work with 50k props
mariusandra Dec 14, 2021
7162157
processing events saves event properties
mariusandra Dec 14, 2021
6434800
fix script
mariusandra Dec 14, 2021
19c12e1
test date format detection
mariusandra Dec 14, 2021
1ff7d94
default enabled
mariusandra Dec 14, 2021
6486d70
only enable event property counter for specific teams
mariusandra Dec 14, 2021
4895947
Merge branch 'master' into event-property-counter
mariusandra Dec 14, 2021
4026f71
eslint fixes
mariusandra Dec 14, 2021
12d3cc4
Merge branch 'master' into event-property-counter
mariusandra Dec 14, 2021
07b20ee
fix logs double-sync noise in tests
mariusandra Dec 14, 2021
9f16444
fix bigint test
mariusandra Dec 14, 2021
c1dc433
don't do tasks that make no sense
mariusandra Dec 14, 2021
114fd2d
remove dead code
mariusandra Dec 14, 2021
523fddd
simpler test setup
mariusandra Dec 16, 2021
97fdd2d
different contraint name
mariusandra Dec 16, 2021
7a5a8a0
refactor team manager
mariusandra Dec 16, 2021
a61884b
greatly simplify the system
mariusandra Dec 16, 2021
20e893b
fetch cached event properties
mariusandra Dec 16, 2021
3ce42cc
fix team manager and timestamps
mariusandra Dec 16, 2021
0d42e92
add cached entry
mariusandra Dec 16, 2021
fbe080e
also don't cache event properties for teams that have it disabled
mariusandra Dec 16, 2021
8e961b8
remove indexes that are not going to be used
mariusandra Dec 16, 2021
aa638e7
Merge branch 'master' into event-property-counter
mariusandra Dec 16, 2021
e7772ab
remove unused imports
mariusandra Dec 16, 2021
8b01a16
blacked
mariusandra Dec 16, 2021
d070faa
remember event properties with a LRU cache
mariusandra Dec 17, 2021
f0f4f36
fix eslint
mariusandra Dec 17, 2021
a39253a
clean up the last bits
mariusandra Dec 17, 2021
cd463f4
move ONE_HOUR to constants
mariusandra Dec 17, 2021
5db0f2d
use sane_repr
mariusandra Dec 17, 2021
c5469cd
fix typo
mariusandra Dec 17, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
clean up the last bits
mariusandra committed Dec 17, 2021

Unverified

This user has not yet uploaded their public signing key.
commit a39253ae8edb1e4bc6f344ad3697b067fdc71ca3
6 changes: 3 additions & 3 deletions plugin-server/src/config/config.ts
Original file line number Diff line number Diff line change
@@ -79,7 +79,7 @@ export function getDefaultConfig(): PluginsServerConfig {
SITE_URL: null,
NEW_PERSON_PROPERTIES_UPDATE_ENABLED_TEAMS: '',
EXPERIMENTAL_EVENTS_LAST_SEEN_ENABLED: true,
EXPERIMENTAL_EVENT_PROPERTY_COUNTER_ENABLED_TEAMS: '',
EXPERIMENTAL_EVENT_PROPERTY_TRACKER_ENABLED_TEAMS: '',
}
}

@@ -139,8 +139,8 @@ export function getConfigHelp(): Record<keyof PluginsServerConfig, string> {
NEW_PERSON_PROPERTIES_UPDATE_ENABLED_TEAMS:
'(advanced) teams for which to run the new person properties update flow on',
EXPERIMENTAL_EVENTS_LAST_SEEN_ENABLED: '(advanced) enable experimental feature to track lastSeenAt',
EXPERIMENTAL_EVENT_PROPERTY_COUNTER_ENABLED_TEAMS:
'(advanced) teams for which to enable experimental feature to count event properties',
EXPERIMENTAL_EVENT_PROPERTY_TRACKER_ENABLED_TEAMS:
'(advanced) teams for which to enable experimental feature to track event properties',
}
}

7 changes: 1 addition & 6 deletions plugin-server/src/types.ts
Original file line number Diff line number Diff line change
@@ -105,7 +105,7 @@ export interface PluginsServerConfig extends Record<string, any> {
SITE_URL: string | null
NEW_PERSON_PROPERTIES_UPDATE_ENABLED_TEAMS: string
EXPERIMENTAL_EVENTS_LAST_SEEN_ENABLED: boolean
EXPERIMENTAL_EVENT_PROPERTY_COUNTER_ENABLED_TEAMS: string
EXPERIMENTAL_EVENT_PROPERTY_TRACKER_ENABLED_TEAMS: string
}

export interface Hub extends PluginsServerConfig {
@@ -767,11 +767,6 @@ export interface EventPropertyType {
id: string
event: string
property: string
property_type: string | null
property_type_format: string | null
total_volume: number | null
created_at: string // DateTime
last_seen_at: string // DateTime
team_id: number
}

3 changes: 0 additions & 3 deletions plugin-server/src/worker/ingestion/process-event.ts
Original file line number Diff line number Diff line change
@@ -78,7 +78,6 @@ export class EventsProcessor {
teamManager: TeamManager
personManager: PersonManager
groupTypeManager: GroupTypeManager
propertyCounterTeams: number[]

constructor(pluginsServer: Hub) {
this.pluginsServer = pluginsServer
@@ -89,8 +88,6 @@ export class EventsProcessor {
this.teamManager = pluginsServer.teamManager
this.personManager = new PersonManager(pluginsServer)
this.groupTypeManager = new GroupTypeManager(pluginsServer.db, this.teamManager, pluginsServer.SITE_URL)
this.propertyCounterTeams =
pluginsServer.EXPERIMENTAL_EVENT_PROPERTY_COUNTER_ENABLED_TEAMS.split(',').map(parseInt)
}

public async processEvent(
16 changes: 8 additions & 8 deletions plugin-server/src/worker/ingestion/team-manager.ts
Original file line number Diff line number Diff line change
@@ -41,10 +41,10 @@ export class TeamManager {
this.lastFlushAt = DateTime.now()

// TODO: #7422 Remove temporary EXPERIMENTAL_EVENTS_LAST_SEEN_ENABLED
// TODO: #7500 Remove temporary EXPERIMENTAL_EVENT_PROPERTY_COUNTER_ENABLED_TEAMS
// TODO: #7500 Remove temporary EXPERIMENTAL_EVENT_PROPERTY_TRACKER_ENABLED_TEAMS
this.experimentalLastSeenAtEnabled = serverConfig.EXPERIMENTAL_EVENTS_LAST_SEEN_ENABLED ?? false
this.propertyCounterTeams = new Set(
(serverConfig.EXPERIMENTAL_EVENT_PROPERTY_COUNTER_ENABLED_TEAMS || '').split(',').map(parseInt)
(serverConfig.EXPERIMENTAL_EVENT_PROPERTY_TRACKER_ENABLED_TEAMS || '').split(',').map(parseInt)
)
}

@@ -177,13 +177,13 @@ export class TeamManager {
if (!this.propertyCounterTeams.has(team.id)) {
return
}
const key = JSON.stringify([team.id, event])
let properties = this.eventPropertiesCache.get(key)
if (!properties) {
properties = new Set()
this.eventPropertiesCache.set(key, properties)
}
for (const property of propertyKeys) {
const key = JSON.stringify([team.id, event])
let properties = this.eventPropertiesCache.get(key)
if (!properties) {
properties = new Set()
this.eventPropertiesCache.set(key, properties)
}
if (!properties.has(property)) {
properties.add(property)
await this.db.postgresQuery(
Copy link
Contributor

Choose a reason for hiding this comment

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

It we could do this insert in a batch instead of per-property 🤔

Copy link
Collaborator Author

@mariusandra mariusandra Dec 17, 2021

Choose a reason for hiding this comment

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

That's pretty much what I was doing in the original implementation, leading to the deadlocks mentioned above. Even if we go from ON CONFLICT DO UPDATE to ON CONFLICT DO NOTHING, I'm going to have to implement code to retry the transactions if the block deadlocks, and all other complexities that come with it.

It would much safer since we're no longer updating the rows that are already in the database, and not running all flush operations at precisely the same moment on every thread, but these deadlocks can't be avoided --> the same new event may come in 10 times in the same millisecond, be handled by 4 threads, which all insert the same properties at precisely the same moment.

Thus, while a bit slower, I think ~20 simple inserts for each new event the first time it's seen will scale much better.

2 changes: 1 addition & 1 deletion plugin-server/tests/shared/process-event.ts
Original file line number Diff line number Diff line change
@@ -121,7 +121,7 @@ export const createProcessEventTests = (
PLUGINS_CELERY_QUEUE: 'test-plugins-celery-queue',
CELERY_DEFAULT_QUEUE: 'test-celery-default-queue',
LOG_LEVEL: LogLevel.Log,
EXPERIMENTAL_EVENT_PROPERTY_COUNTER_ENABLED_TEAMS: '2',
EXPERIMENTAL_EVENT_PROPERTY_TRACKER_ENABLED_TEAMS: '2',
...(extraServerConfig ?? {}),
...(additionalProps ?? {}),
})
2 changes: 1 addition & 1 deletion plugin-server/tests/worker/ingestion/team-manager.test.ts
Original file line number Diff line number Diff line change
@@ -23,7 +23,7 @@ describe('TeamManager()', () => {

beforeEach(async () => {
;[hub, closeHub] = await createHub({
EXPERIMENTAL_EVENT_PROPERTY_COUNTER_ENABLED_TEAMS: '2',
EXPERIMENTAL_EVENT_PROPERTY_TRACKER_ENABLED_TEAMS: '2',
})
await resetTestDatabase()
teamManager = hub.teamManager