Skip to content

Comments

Fix HPPS migrations for restricted hosting environments#7899

Open
donnapep wants to merge 22 commits intotrunkfrom
fix/hpps-migration-time-budget
Open

Fix HPPS migrations for restricted hosting environments#7899
donnapep wants to merge 22 commits intotrunkfrom
fix/hpps-migration-time-budget

Conversation

@donnapep
Copy link
Contributor

@donnapep donnapep commented Feb 24, 2026

Summary

  • Remove set_time_limit(0) from migration scheduler and replace with time-budgeted processing (default 20s, filterable)
  • Reduce migration batch sizes (Student Progress: 1000→250 rows/run, Quiz: 100→50)
  • Add retry logic for failed migrations (3 retries before marking as failed)
  • Remove Settings UI gate that blocked HPPS on hosts where set_time_limit is disabled
  • Fix typo: LARST_COMMENT_ID_OPTION_NAMELAST_COMMENT_ID_OPTION_NAME

Context

HPPS (High-Performance Progress Storage) Stage 4 — Phase 1. Migrations previously required set_time_limit(0) which fails on WP.com Atomic and restricted hosting environments. This PR makes migrations work within normal PHP execution limits.

New filters

  • sensei_hpps_migration_time_budget (float, default 20.0) — seconds per migration run
  • sensei_hpps_migration_max_retries (int, default 3) — retry attempts before failing
  • sensei_hpps_student_progress_batch_size (int, default 50)
  • sensei_hpps_student_progress_batch_count (int, default 5)
  • sensei_hpps_quiz_migration_batch_size (int, default 50)

Test plan

  • Run migration tests: npm run test-php:wp-env -- --filter "Migration_Abstract_Test|Migration_Job_Test|Migration_Job_Scheduler_Test|Student_Progress_Migration_Test|Quiz_Migration_Test"
  • Verify Settings UI shows HPPS toggle on environment where set_time_limit is disabled
  • Test migration completes successfully with reduced batch sizes
  • Test retry logic: migration recovers from transient Action Scheduler failure

donnapep and others added 9 commits February 24, 2026 08:22
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings February 24, 2026 14:24
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR makes HPPS (High-Performance Progress Storage) migrations compatible with restricted hosting environments (like WP.com Atomic) where set_time_limit() is disabled. The changes implement time-budgeted processing, reduce batch sizes to work within normal PHP execution limits, and add retry logic for transient failures.

Changes:

  • Implemented time-budgeted migration processing with 80% threshold and default 20-second budget
  • Reduced migration batch sizes: Student Progress from 1000 to 250 rows/run (50×5), Quiz from 100 to 50 submissions/run
  • Added retry logic with default 3 attempts before marking migrations as failed
  • Removed Settings UI restrictions requiring set_time_limit() support
  • Fixed typo in constant name with backward-compatible alias

Reviewed changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
includes/internal/migration/class-migration-abstract.php Added time budget tracking with set_time_budget() and is_time_exceeded() methods using 80% threshold
includes/internal/migration/class-migration-job.php Added set_time_budget() method to delegate budget setting to underlying migration
includes/internal/migration/class-migration-job-scheduler.php Removed set_time_limit(0), added retry logic with configurable max retries, applies time budget filter to jobs
includes/internal/migration/migrations/class-student-progress-migration.php Reduced batch size/count defaults, added filterable batch configuration, checks time budget in insert_with_batches(), fixed typo with backward compatibility
includes/internal/migration/migrations/class-quiz-migration.php Reduced batch size default, added filterable batch size, checks time budget in main processing loop
includes/class-sensei-settings.php Removed set_time_limit() capability check that previously disabled HPPS feature on restricted hosts
tests/unit-tests/internal/migration/test-class-migration-abstract.php New test file covering time budget functionality
tests/unit-tests/internal/migration/test-class-migration-job.php Added test for time budget delegation
tests/unit-tests/internal/migration/test-class-migration-job-scheduler.php Added tests for time budget application, retry logic, and retry count cleanup
tests/unit-tests/internal/migration/migrations/test-class-student-progress-migration.php Added tests for reduced batch sizes, filterable batch configuration, and time budget behavior
tests/unit-tests/internal/migration/migrations/test-class-quiz-migration.php Added tests for reduced batch size, filterable batch size, and time budget behavior
.gitignore Added .worktrees for git worktree support
Comments suppressed due to low confidence (1)

includes/internal/migration/migrations/class-quiz-migration.php:101

  • The return value documentation states "The number of quiz submissions migrated" but line 101 returns count( $comments ) which is the number of comments processed, not necessarily the number of quiz submissions successfully migrated. When a comment has no associated quiz (line 81-84), it's counted in the returned value even though no submission was migrated. Line 94 returns $processed which has the same issue.

Consider either updating the documentation to say "The number of comments processed" or tracking and returning the actual count of successfully migrated submissions.

	 * @return int The number of quiz submissions migrated.
	 */
	public function run( bool $dry_run = true ) {
		$this->dry_run = $dry_run;

		$comments = $this->get_comments();
		if ( ! $comments ) {
			return 0;
		}

		$quiz_data = $this->get_quiz_data( $comments );
		$processed = 0;
		foreach ( $comments as $comment ) {
			$submission_id = $this->insert_quiz_submission( $comment, $quiz_data );
			if ( ! $submission_id ) {
				++$processed;
				continue;
			}

			$answer_ids = $this->insert_quiz_answers( $comment, $quiz_data, $submission_id );
			$this->insert_quiz_grades( $comment, $quiz_data, $answer_ids );

			++$processed;

			// Update cursor and stop if time budget exceeded.
			if ( $this->is_time_exceeded() && $processed < count( $comments ) ) {
				update_option( self::LAST_COMMENT_ID_OPTION_NAME, $comment->comment_ID );
				return $processed;
			}
		}

		$last_comment_id = end( $comments )->comment_ID;
		update_option( self::LAST_COMMENT_ID_OPTION_NAME, $last_comment_id );

		return count( $comments );

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@donnapep donnapep self-assigned this Feb 24, 2026
@donnapep donnapep added this to the 4.25.3 milestone Feb 24, 2026
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@donnapep donnapep added the Hooks This change adds or modifies one or more hooks. label Feb 24, 2026
donnapep and others added 2 commits February 24, 2026 10:42
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When insert_with_batches() exits early due to time budget, the cursor
was still set to the last fetched comment ID, skipping rows that were
never inserted. Now the cursor only advances if the time budget has
not been exceeded. INSERT IGNORE handles duplicates on re-run safely.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link

Copilot AI commented Feb 24, 2026

@donnapep I've opened a new pull request, #7900, to work on those changes. Once the pull request is ready, I'll request review from you.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

donnapep and others added 7 commits February 24, 2026 11:02
Add missing @SInCE, @return, and @var tags to new code introduced in
this PR. Fix incorrect doc block description on Quiz_Migration constant.
Add doc blocks to previously undocumented status constants.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove tests for batch size defaults, batch size filters, and backward
compat constant alias. Add descriptive messages to time budget assertions.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
donnapep and others added 3 commits February 24, 2026 15:07
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Instead of not advancing the cursor when time is exceeded, track the
last fully processed comment ID and always advance to it. This prevents
infinite rescheduling when a batch consistently exceeds the time budget.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Hooks This change adds or modifies one or more hooks.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants