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

Add time_spent to UserLevels #31231

Closed
wants to merge 1 commit into from
Closed

Add time_spent to UserLevels #31231

wants to merge 1 commit into from

Conversation

dmcavoy
Copy link
Contributor

@dmcavoy dmcavoy commented Oct 11, 2019

Description

Completes step one of the plan for Recording Time Spent for Validated Levels in the Recording Time Spent Plan.

Links

Reviewer Checklist:

  • Tests provide adequate coverage
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

Copy link
Contributor

@wjordan wjordan left a comment

Choose a reason for hiding this comment

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

I'd like some confirmation that this migration will have no production impact before we attempt to apply this migration to our production DB. For reference, the last time we added a column to user_levels it took about 4 hours (and that was in 2016 when this table was much smaller). We also proactively queued writes/updates to the table to reduce impact (using a queue system we no longer maintain).

I had previously speculated that adding a column might be quick using Aurora's Fast DDL feature, but it has never been used before on our database, it must be enabled first through a parameter change, and the documentation also states that "We don't recommended using fast DDL for production DB clusters".

If we do choose to use the Fast DDL feature for this migration, it will need thorough testing/validation (e.g., on a DB clone) before we can depend on it in production.

@davidsbailey
Copy link
Member

@dmcavoy , do we have a sense of what fraction of user levels will use this field? for example, if we only use this in CSP it might be a small fraction relative to HoC + CSF, or if we only use it on assessments, etc. if it is a small fraction, it might make sense to use a different table, although I guess I don't know for sure that that's true in Aurora.

@wjordan
Copy link
Contributor

wjordan commented Oct 11, 2019

if it is a small fraction, it might make sense to use a different table, although I guess I don't know for sure that that's true in Aurora.

Yes- if this information is only going to be used by a small fraction of user-level activity, it makes sense (= more resource-efficient at runtime and also easier to operationally manage through migrations) to use a separate table to store/query the data rather than adding an extra column to the user_levels table.

@Erin007
Copy link
Contributor

Erin007 commented Oct 11, 2019

if it is a small fraction, it might make sense to use a different table, although I guess I don't know for sure that that's true in Aurora.

Yes- if this information is only going to be used by a small fraction of user-level activity, it makes sense (= more resource-efficient at runtime and also easier to operationally manage through migrations) to use a separate table to store/query the data rather than adding an extra column to the user_levels table.

My understanding from the Product team from the initial planning is that we're interested first in time spent for validated CSF levels for the specific purpose of including it on the progress tab; but that we'll also want this information for other level types in the future.

@davidsbailey
Copy link
Member

davidsbailey commented Oct 14, 2019

If the number of levels which will use this will be small, then let's use a different table. I'd suggest
user_level_info EDIT: user_level_infos for the name, unless anyone can think of a better one.

@dmcavoy dmcavoy closed this Oct 15, 2019
@dmcavoy dmcavoy deleted the time-spent-add-column branch October 15, 2019 15:56
@dmcavoy dmcavoy restored the time-spent-add-column branch October 15, 2019 17:16
@dmcavoy dmcavoy deleted the time-spent-add-column branch October 16, 2019 11:40
@davidsbailey davidsbailey mentioned this pull request Oct 29, 2019
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants