Skip to content

Index Device by user and last activity date - #8314

Draft
maxkpower wants to merge 1 commit into
mainfrom
dirt/prototype/adoption-report/device-index
Draft

Index Device by user and last activity date#8314
maxkpower wants to merge 1 commit into
mainfrom
dirt/prototype/adoption-report/device-index

Conversation

@maxkpower

Copy link
Copy Markdown

🎟️ Tracking

PM-35924

📔 Objective

Adds IX_Device_UserId_LastActivityDate_Type to dbo.Device, serving the member adoption report's two per-member Device probes.

  • dbo.Device is a large production table. The DDL is guarded by IF NOT EXISTS (SELECT NULL FROM sys.indexes ...) and deliberately omits ONLINE = ON, because self-hosted runs SQL Server Express (util/Setup/EnvironmentFileBuilder.cs:31), which does not support online index builds.
  • Three key columns rather than INCLUDE ([Type]): measured identical on both probes, and Pomelo and SQLite ignore INCLUDE, so the key form is the only one that lands the same shape on all four providers.
  • Measured on a 600k-row synthetic table: the MAX([LastActivityDate]) probe drops to a single backward seek, and carrying [Type] removes the key lookup per device row from the extension probe (36,734 to 16,022 logical reads).
  • 12,364 of the ~12,500 lines are dotnet ef output. Reviewable surface: git diff main... -- . ':!*.Designer.cs'.

Deployment note: this runs offline inside DbUp's transaction under a 5 minute timeout (MigratorConstants.cs:8), and DatabaseMigrationHostedService retries 10 times at 20 second intervals. On a production-sized Device table that can block device writes and then fail with no progress. Raise MigrationExecutionTimeoutSeconds for the cloud run, or use --no-transaction.

Bottom of a 3 PR stack. data-access sits on this.

📸 Screenshots

No user-visible change.

@codecov

codecov Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.56%. Comparing base (cb1f141) to head (b5eca58).

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #8314       +/-   ##
===========================================
+ Coverage   17.70%   69.56%   +51.86%     
===========================================
  Files        1480     2471      +991     
  Lines       64163   105935    +41772     
  Branches     5148     9601     +4453     
===========================================
+ Hits        11359    73695    +62336     
+ Misses      52607    29777    -22830     
- Partials      197     2463     +2266     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@maxkpower maxkpower added the ai-review Request a Claude code review label Sep 4, 2026
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the additive IX_Device_UserId_LastActivityDate_Type index across all four supported providers. The MSSQL track is consistent: src/Sql/dbo/Tables/Device.sql and util/Migrator/DbScripts/2026-09-03_00_AddDeviceUserIdLastActivityDateIndex.sql declare the same three key columns in the same order, the migration filename follows YYYY-MM-DD_##_Description.sql, the sys.indexes guard makes it idempotent, and omitting ONLINE = ON matches the repo's index convention. The EF track matches: DeviceEntityTypeConfiguration adds the index, all three provider migrations create and drop the identical name and column list, and each DatabaseContextModelSnapshot.cs diff is limited to the three expected lines with no unrelated model drift. The change is purely additive with a working Down, so it carries no EDD rollback or backwards-compatibility exposure, and it introduces no runtime logic requiring unit tests.

@maxkpower
maxkpower force-pushed the dirt/prototype/adoption-report/device-index branch from b5eca58 to 674b2af Compare September 8, 2026 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant