fix: count one app open per account per app per hour (#4152)

* fix: count one app open per account per app per day

`app_opens` is read two ways at once: `open_count` is a row count and
`user_count` is distinct accounts. A repeated open moves the first
without the first moving the second, and the incentive report prices
opens.

An open is now marked as it is recorded and skipped for the rest of the
day, before both the row and the event ClickHouse ingests, so the two
stores agree. The marker is set only when absent, so the first open of
the day wins a race; an unreachable marker counts the open, since
counting twice beats not counting.

* fix: count one app open per account per app per day, at the count

Dropped the row-suppression approach. `app_opens` is not only a counter:
`getRecentAppOpens` orders the desktop's recent list by `MAX(_id)`, and
`app.opened` is the socket event other tabs reorder on. Suppressing the
repeat row froze both at the first open of the day.

Every open is recorded as before. `open_count` counts each account once
per day instead, which is the figure `user_count` is read against.

`countDistinctUserDays` folds the pair into one number because
`COUNT(DISTINCT a, b)` is MySQL-only, and `dayBucket` covers MySQL's
decimal division. ClickHouse has uniqExact over the tuple.

Note the figure's meaning changes at deploy: a period spanning it mixes
rows with account-days, so a comparison across that point is not one.

* fix: count one app open per account per app per hour

Review feedback: a day is too long a window for the repeat to collapse
into. The period is now an argument rather than a baked-in 86400, held in
one constant on AppStore beside the other store policy numbers, so the SQL
and ClickHouse sites cannot drift and a future change is one line.

The fold `COUNT(DISTINCT user * N + bucket)` is only injective while the
bucket index stays under N. Hourly buckets are 24x the day index and pass
the old 1000000 in 2084, so N moves to 10000000.

Buckets are clock hours, not a rolling window, so two opens a minute apart
either side of the hour still count twice — tested, and the same property
the day version had at UTC midnight.

The detailed-stats fixture gave its repeat open a 60s offset, which under
hourly buckets would have counted twice whenever the suite ran in the first
minute of an hour. It now shares a timestamp with the open it repeats.
This commit is contained in:
Juan Fernando Castro authored and GitHub committed 2026-10-14 11:54:17 -04:00
1 parent ff815f730d
commit e32bfa839d
4 files changed
+140 -23

No files matched your search

@@ -330,6 +330,30 @@ export class AbstractDatabaseClient extends PuterClient {
});
}
/**
* Whole `periodSeconds` buckets since the epoch, from a unix-seconds
* column. MySQL's `/` yields a decimal, which would split a bucket.
*/
periodBucket(secondsExpression: string, periodSeconds: number): string {
return this.case({
mysql: `(${secondsExpression} DIV ${periodSeconds})`,
otherwise: `(${secondsExpression} / ${periodSeconds})`,
});
}
/**
* Counts each (`userExpression`, bucket) pair once. `COUNT(DISTINCT a, b)`
* is MySQL-only, so the pair folds into one number — injective while the
* bucket stays under the multiplier, which hourly reaches in the 2980s.
*/
countDistinctUserPeriods(
userExpression: string,
secondsExpression: string,
periodSeconds: number,
): string {
return `COUNT(DISTINCT ${userExpression} * 10000000 + ${this.periodBucket(secondsExpression, periodSeconds)})`;
}
nullCoalesce(...expressions: readonly string[]): string {
if (expressions.length === 0) {
throw new Error('nullCoalesce requires at least one expression');
@@ -338,6 +338,42 @@ describe('AppController POST /rao', () => {
expect(rows).toHaveLength(1);
});
it('keeps every open as a row, and counts one per account per hour', async () => {
const owner = await makeUser();
const other = await makeUser();
const app = await createApp(owner.actor);
const open = async (user: { actor: Actor; userId: number }) => {
const { res } = makeRes();
await withActor(user.actor, () =>
callRoute(
'post',
'/rao',
makeReq({ body: { app_uid: app.uid }, actor: user.actor }),
res,
),
);
};
for (let i = 0; i < 5; i++) await open(owner);
await open(other);
await server.controllers.apps.drainPendingAppOpens();
// Every open is still a row: the recent-apps list orders on them.
const rows = (await server.clients.db.read(
'SELECT `user_id` FROM `app_opens` WHERE `app_uid` = ?',
[app.uid],
)) as Array<{ user_id: number }>;
expect(rows).toHaveLength(6);
// The count is accounts-per-hour, so repeats do not move it.
const stats = await server.stores.app.getAppsStats([app.uid]);
expect(stats.get(app.uid)).toMatchObject({
open_count: 2,
user_count: 2,
});
});
// `app_opens` is analytics: the response carries nothing derived from it
// and the client never awaits the post. Holding the response open for a
// primary write put that write's latency inside the app launch it was
+8 -6
View File
@@ -62,6 +62,8 @@ const OLD_APP_NAME_TTL_MONTHS = 3;
const BULK_QUERY_CHUNK_SIZE = 200;
// Placeholders per index_url `IN (…)` chunk; under SQLite's old 999 default.
const INDEX_URL_CHUNK_SIZE = 900;
// Window an account's repeat opens of one app collapse into a single count.
const APP_OPEN_COUNT_PERIOD_SECONDS = 60 * 60;
// Top-level all-time open/user counts: hot path, slow to compute. Cached
// lazily on read (pipelined MGET on every app list/read; misses query the
@@ -1446,7 +1448,7 @@ export class AppStore extends PuterStore {
const res = await clickhouse.query({
query: `
SELECT app_uid,
count(_id) AS open_count,
uniqExact((user_id, intDiv(ts, ${APP_OPEN_COUNT_PERIOD_SECONDS}))) AS open_count,
count(DISTINCT user_id) AS user_count
FROM app_opens
WHERE app_uid IN {uids:Array(String)}
@@ -1468,7 +1470,7 @@ export class AppStore extends PuterStore {
const placeholders = uids.map(() => '?').join(',');
const rows = await this.clients.db.read(
`SELECT app_uid,
COUNT(_id) AS open_count,
${this.clients.db.countDistinctUserPeriods('user_id', 'ts', APP_OPEN_COUNT_PERIOD_SECONDS)} AS open_count,
COUNT(DISTINCT user_id) AS user_count
FROM app_opens
WHERE app_uid IN (${placeholders})
@@ -1495,7 +1497,7 @@ export class AppStore extends PuterStore {
}
const res = await clickhouse.query({
query: `
SELECT count(_id) AS open_count,
SELECT uniqExact((user_id, intDiv(ts, ${APP_OPEN_COUNT_PERIOD_SECONDS}))) AS open_count,
count(DISTINCT user_id) AS user_count
FROM app_opens
WHERE app_uid = {appUid:String}
@@ -1518,7 +1520,7 @@ export class AppStore extends PuterStore {
: [appUid];
const where = timeRange ? 'AND ts >= ? AND ts < ?' : '';
const rows = await this.clients.db.read(
`SELECT COUNT(_id) AS open_count,
`SELECT ${this.clients.db.countDistinctUserPeriods('user_id', 'ts', APP_OPEN_COUNT_PERIOD_SECONDS)} AS open_count,
COUNT(DISTINCT user_id) AS user_count
FROM app_opens
WHERE app_uid = ? ${where}`,
@@ -1544,7 +1546,7 @@ export class AppStore extends PuterStore {
const res = await clickhouse.query({
query: `
SELECT ${groupBy} AS period,
count(_id) AS open_count,
uniqExact((user_id, intDiv(ts, ${APP_OPEN_COUNT_PERIOD_SECONDS}))) AS open_count,
count(DISTINCT user_id) AS user_count
FROM app_opens
WHERE app_uid = {appUid:String}
@@ -1579,7 +1581,7 @@ export class AppStore extends PuterStore {
});
const rows = await this.clients.db.read(
`SELECT ${periodExpr} AS period,
COUNT(_id) AS open_count,
${this.clients.db.countDistinctUserPeriods('user_id', 'ts', APP_OPEN_COUNT_PERIOD_SECONDS)} AS open_count,
COUNT(DISTINCT user_id) AS user_count
FROM app_opens
WHERE app_uid = ? AND ts >= ? AND ts < ?
+72 -17
View File
@@ -77,7 +77,7 @@ describe('AppStore app stats (cache-on-read)', () => {
it('computes all-time open/unique-user counts from SQL when no ClickHouse client is registered', async () => {
const uid = freshUid();
// 3 opens across 2 distinct users.
// 3 rows across 2 users, two of them the same user within one hour.
await insertOpen(uid, 1, 1700000000);
await insertOpen(uid, 1, 1700000100);
await insertOpen(uid, 2, 1700000200);
@@ -85,12 +85,39 @@ describe('AppStore app stats (cache-on-read)', () => {
const stats = await appStore.getAppsStats([uid]);
expect(stats.get(uid)).toEqual({
open_count: 3,
// An account's repeat opens in an hour count once.
open_count: 2,
user_count: 2,
referral_count: null,
});
});
it('counts the same account again in a later hour', async () => {
const uid = freshUid();
const hour = 3600;
await insertOpen(uid, 1, 1700000000);
await insertOpen(uid, 1, 1700000000 + 30);
await insertOpen(uid, 1, 1700000000 + hour);
await insertOpen(uid, 1, 1700000000 + 2 * hour);
expect((await appStore.getAppsStats([uid])).get(uid)).toMatchObject({
open_count: 3,
user_count: 1,
});
});
it('buckets on the clock hour rather than a rolling window', async () => {
const uid = freshUid();
// 1699999200 starts an hour; these straddle its end a second apart.
await insertOpen(uid, 1, 1699999200 + 3599);
await insertOpen(uid, 1, 1699999200 + 3600);
expect((await appStore.getAppsStats([uid])).get(uid)).toMatchObject({
open_count: 2,
user_count: 1,
});
});
it('caches counts on read with an autoexpire TTL', async () => {
const uid = freshUid();
await insertOpen(uid, 7, 1700000000);
@@ -1425,10 +1452,10 @@ describe('AppStore detailed stats', () => {
db = server.clients.db;
await clearAppCache(server.clients.redis);
// Three opens right now from two distinct users.
// Two users; the repeat shares a timestamp so it is always same-hour.
for (const [userId, offset] of [
[1, 0],
[1, 60],
[1, 0],
[2, 120],
]) {
await db.write(
@@ -1445,7 +1472,8 @@ describe('AppStore detailed stats', () => {
it('computes all-time totals from SQL by default', async () => {
const stats = await appStore.getAppStatsDetailed(APP_UID);
expect(stats).toEqual({
open_count: 3,
// Three rows, but two of them one account in one hour.
open_count: 2,
user_count: 2,
referral_count: null,
});
@@ -1471,7 +1499,8 @@ describe('AppStore detailed stats', () => {
const stats = await appStore.getAppStatsDetailed(APP_UID, {
period,
});
expect(stats.open_count).toBe(3);
// The fixture's three rows are two accounts in one hour.
expect(stats.open_count).toBe(2);
expect(stats.user_count).toBe(2);
});
@@ -1498,7 +1527,7 @@ describe('AppStore detailed stats', () => {
const stats = await appStore.getAppStatsDetailed(APP_UID, {
period: 'not-a-period',
});
expect(stats.open_count).toBe(3);
expect(stats.open_count).toBe(2);
});
it('rejects an unsupported grouping', async () => {
@@ -1516,7 +1545,7 @@ describe('AppStore detailed stats', () => {
grouping: 'day',
});
expect(stats.open_count).toBe(3);
expect(stats.open_count).toBe(2);
expect(stats.user_count).toBe(2);
const open = stats.grouped_stats.open_count;
// A 7-day window yields 8 day buckets (inclusive of both ends).
@@ -1524,7 +1553,7 @@ describe('AppStore detailed stats', () => {
expect(open.every((b) => typeof b.count === 'number')).toBe(true);
const today = new Date().toISOString().slice(0, 10);
const todayBucket = open.find((b) => b.period === today);
expect(todayBucket?.count).toBe(3);
expect(todayBucket?.count).toBe(2);
// Every earlier bucket is an explicit zero, not a gap.
expect(
open.filter((b) => b.period !== today).every((b) => b.count === 0),
@@ -1538,7 +1567,7 @@ describe('AppStore detailed stats', () => {
period: 'today',
grouping,
});
expect(stats.open_count).toBe(3);
expect(stats.open_count).toBe(2);
expect(stats.grouped_stats.open_count.length).toBeGreaterThan(0);
expect(stats.grouped_stats.user_count.length).toBe(
stats.grouped_stats.open_count.length,
@@ -1555,7 +1584,7 @@ describe('AppStore detailed stats', () => {
period: '30d',
grouping: 'week',
});
expect(stats.open_count).toBe(3);
expect(stats.open_count).toBe(2);
expect(stats.grouped_stats.open_count.length).toBeGreaterThan(0);
});
@@ -1830,13 +1859,23 @@ describe('AppStore findCanonicalUidByIndexUrlCandidates', () => {
it('prefers the private row, then the oldest match', async () => {
const url = `https://canon-${Math.random().toString(36).slice(2, 10)}.example.com/`;
const pub = await appStore.create(
{ name: `pub-${Math.random().toString(36).slice(2, 8)}`, title: 't', index_url: url },
{
name: `pub-${Math.random().toString(36).slice(2, 8)}`,
title: 't',
index_url: url,
},
{ ownerUserId: 1 },
);
const privUid = `app-${Math.random().toString(36).slice(2, 10)}`;
await server.clients.db.write(
'INSERT INTO `apps` (`uid`, `name`, `title`, `index_url`, `is_private`) VALUES (?, ?, ?, ?, ?)',
[privUid, `priv-${Math.random().toString(36).slice(2, 8)}`, 't', url, 1],
[
privUid,
`priv-${Math.random().toString(36).slice(2, 8)}`,
't',
url,
1,
],
);
const uid = await appStore.findCanonicalUidByIndexUrlCandidates([url]);
@@ -1845,8 +1884,12 @@ describe('AppStore findCanonicalUidByIndexUrlCandidates', () => {
});
it('returns null for an empty, non-array, or non-matching candidate list', async () => {
expect(await appStore.findCanonicalUidByIndexUrlCandidates([])).toBeNull();
expect(await appStore.findCanonicalUidByIndexUrlCandidates(null)).toBeNull();
expect(
await appStore.findCanonicalUidByIndexUrlCandidates([]),
).toBeNull();
expect(
await appStore.findCanonicalUidByIndexUrlCandidates(null),
).toBeNull();
expect(
await appStore.findCanonicalUidByIndexUrlCandidates([
'https://nothing-here.example.com/',
@@ -1903,13 +1946,25 @@ describe('AppStore listIndexUrlWinners', () => {
const priv1Uid = `app-${Math.random().toString(36).slice(2, 10)}`;
await server.clients.db.write(
'INSERT INTO `apps` (`uid`, `name`, `title`, `index_url`, `is_private`) VALUES (?, ?, ?, ?, ?)',
[priv1Uid, `priv1-${Math.random().toString(36).slice(2, 8)}`, 't', url, 1],
[
priv1Uid,
`priv1-${Math.random().toString(36).slice(2, 8)}`,
't',
url,
1,
],
);
const priv1 = await appStore.getByUid(priv1Uid);
const priv2Uid = `app-${Math.random().toString(36).slice(2, 10)}`;
await server.clients.db.write(
'INSERT INTO `apps` (`uid`, `name`, `title`, `index_url`, `is_private`) VALUES (?, ?, ?, ?, ?)',
[priv2Uid, `priv2-${Math.random().toString(36).slice(2, 8)}`, 't', url, 1],
[
priv2Uid,
`priv2-${Math.random().toString(36).slice(2, 8)}`,
't',
url,
1,
],
);
const priv2 = await appStore.getByUid(priv2Uid);