Select Dismissal Unexpected Behavior

There was a new feature added in May Selective dismissal of New and Unread topics. We recently just upgraded to the latest discourse, and looking at the code and our DB logs, it seems like when users “Dismiss New” without selecting (ie. just bulk dismiss) we pass in tracked = false and nothing else in the request parameters to reset-new.

image

Inspecting the PR changes I think we end up here when we try to figure out our topic_scope FEATURE: Allow selective dismissal of new and unread topics (#12976) · discourse/discourse@7a79bd7 · GitHub

and because we don’t pass in any topic_ids we skip the if statement and automatically jump to L989

We run a fairly large discourse server, and I think the notion of just having all Topics is pretty expensive as the query basically generates

LEFT JOIN topic_users ON topic_users.topic_id = topics.id AND topic_users.user_id = xxx WHERE "topics"."deleted_at" IS NULL AND "topics"."id" IN (1, ... 1000000) AND (topics.created_at >= 'date') AND (topic_users.last_read_post_number IS NULL) AND (topics.archetype <> 'private_message') ORDER BY topics.created_at DESC LIMIT 500

the IN clause has I believe almost every single topic in the 1M+ range.

Can we default the bulk select to just pass in the topic ids instead of defaulting to all possible Topics?

3 Likes

@martin Hey sorry for the notification, looks like you pushed the commit in question linked above.

Any chance you could check if this seems accurate as the root cause of what we’re seeing? We’re seeing huge filter lists in the queries (~a million topic ids) in the default case of “dismiss all new”. (our forum hosts tons of Discourse topics)

We could help with a PR to resolve it if Discourse team has no priority for this, but I doubt we are the only ones affected by this, might also be impacting DB performance of some of your customers.

1 Like

Thanks @forkythetoy and @Hooksmith for bringing this to my attention, I confirmed this is a huge query even here on meta. I will write a fix for this today, it should be as simple as only using the topic IDs for the topics that show on the New list for a user, rather than all topics ever. Will post back here when I have the patch.

2 Likes

I merged this fix today:

2 Likes