Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Add mau_trial_days config option #3739

Closed
wants to merge 11 commits into from
Closed

Add mau_trial_days config option #3739

wants to merge 11 commits into from

Conversation

ara4n
Copy link
Member

@ara4n ara4n commented Aug 22, 2018

Lets users have N days before they get counted as an MAU

Lets users have N days before they get counted as an MAU
@erikjohnston
Copy link
Member

Won't this mean that if 1000 user sign up on the first day, with an MAU limit of 100, that those 1000 users will still be able to use the service after the 2 days are up?

@ara4n
Copy link
Member Author

ara4n commented Aug 22, 2018

oops, totally missed that. have changed it so that we explicitly track whether MAUs are trial or not, and if they're trial, we only let them be promoted to non-trial users if there's enough MAU headroom.

@ara4n
Copy link
Member Author

ara4n commented Aug 22, 2018

right, this looks like it should work (although I haven't extended the tests to cover the concept of trial users yet, but at least if you don't specify a trial duration it works as well as it did before)...

The changes boil down to:

  • Track whether a given user is considered as a trial user in the MAU table via a boolean.
  • Trial users (who are within their trial period) and proper MAU users are considered as valid users.
  • The reap task tries to promote trial users to be non-trial if they have been active for more than the trial period... but only if there is sufficient MAU headroom. It prioritises trial users who have been active for longer (i.e. older users), and the rest end up in purgatory until the HS increases its max MAU.

I think this gives us what is needed?

@erikjohnston PTAL. @rxl881, @AmandineLP cc'ing fyi.

@ara4n
Copy link
Member Author

ara4n commented Aug 22, 2018

@neilisfragile suggests an easier way of doing this would be to just only insert users into the MAU table if they've been around for more than N trial days. This seems a lot simpler, and we could measure 'been around' based on looking at when the account was registered. I'm not seeing an obvious flaw, but before i charge off and implement that one, can someone sanity check it?

ara4n added a commit that referenced this pull request Aug 22, 2018
only consider users MAU after they've been around N days.
This is an alternative implementation to #3739
as suggested by @neilisfragile, which is much simpler as you just hold off adding
users to the MAU table until they've been active for more than N days.
@ara4n
Copy link
Member Author

ara4n commented Aug 22, 2018

I went and implemented @neilisfragile's suggestion over at #3744 anyway; I can't seen an obvious problem with it.

@erikjohnston you get to discard one (or more) of these, i think ;P

@ara4n
Copy link
Member Author

ara4n commented Aug 24, 2018

obsoleted by #3749 - thanks @erikjohnston for picking it up.

@ara4n ara4n closed this Aug 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants