-
Notifications
You must be signed in to change notification settings - Fork 25
Better handlng of transient validation errors #111
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,26 +1,27 @@ | ||
| class ValidationJob < ApplicationJob | ||
| queue_as :validation | ||
|
|
||
| # Retry up to 3 times with fixed delays to avoid the exponentially_longer issue | ||
| retry_on StandardError, wait: 5.seconds, attempts: 3 | ||
| # Import TransientValidationError from BlockValidator | ||
| TransientValidationError = BlockValidator::TransientValidationError | ||
|
|
||
| # Only retry transient errors, not all StandardError | ||
| retry_on TransientValidationError, | ||
| wait: ENV.fetch('VALIDATION_RETRY_WAIT_SECONDS', 5).to_i.seconds, | ||
| attempts: ENV.fetch('VALIDATION_TRANSIENT_RETRIES', 5).to_i | ||
|
|
||
| def perform(l1_block_number, l2_block_hashes) | ||
| start_time = Time.current | ||
|
|
||
| begin | ||
| # Use the unified ValidationResult model to validate and save | ||
| validation_result = ValidationResult.validate_and_save(l1_block_number, l2_block_hashes) | ||
|
|
||
| elapsed_time = Time.current - start_time | ||
| Rails.logger.info "ValidationJob: Block #{l1_block_number} validation completed in #{elapsed_time.round(3)}s" | ||
|
|
||
| rescue => e | ||
| Rails.logger.error "ValidationJob: Validation failed for block #{l1_block_number}: #{e.message}" | ||
| # ValidationResult.validate_and_save will: | ||
| # 1. Create ValidationResult with success: true (job succeeds) | ||
| # 2. Create ValidationResult with success: false (job succeeds - real validation failure found) | ||
| # 3. Raise TransientValidationError (job retries via retry_on, then fails if exhausted) | ||
| ValidationResult.validate_and_save(l1_block_number, l2_block_hashes) | ||
|
|
||
| # Validation failure will be detected by import process via database query | ||
| elapsed_time = Time.current - start_time | ||
| Rails.logger.info "ValidationJob: Block #{l1_block_number} validation completed in #{elapsed_time.round(3)}s" | ||
|
|
||
| # Re-raise to trigger retry mechanism | ||
| raise e | ||
| end | ||
| # Job completes successfully for cases 1 & 2 | ||
| # TransientValidationError will be handled by retry_on automatically | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,6 +32,14 @@ def self.validation_gaps(start_block, end_block) | |
| connection.execute(sql, [start_block, end_block]).map { |row| row['missing_block'] } | ||
| end | ||
|
|
||
| # Faster method that just counts gaps without listing them all | ||
| def self.validation_gap_count(start_block, end_block) | ||
| # Count how many blocks are missing in the range | ||
| expected_count = end_block - start_block + 1 | ||
| validated_count = where(l1_block: start_block..end_block).count | ||
| expected_count - validated_count | ||
| end | ||
|
|
||
| def self.validation_stats(since: 1.hour.ago) | ||
| results = where('validated_at >= ?', since) | ||
| total = results.count | ||
|
|
@@ -60,11 +68,13 @@ def self.validate_and_save(l1_block_number, l2_block_hashes) | |
| start_time = Time.current | ||
| block_result = validator.validate_l1_block(l1_block_number, l2_block_hashes) | ||
|
|
||
| # Store comprehensive validation result with full debugging data | ||
| validation_result = create_or_find_by(l1_block: l1_block_number) do |vr| | ||
| vr.success = block_result.success | ||
| vr.error_details = block_result.errors | ||
| vr.validation_stats = { | ||
| # Find or initialize - idempotent for re-runs | ||
| validation_result = find_or_initialize_by(l1_block: l1_block_number) | ||
|
|
||
| validation_result.assign_attributes( | ||
| success: block_result.success, | ||
| error_details: block_result.errors, | ||
| validation_stats: { | ||
| # Basic stats | ||
| success: block_result.success, | ||
| l1_block: l1_block_number, | ||
|
|
@@ -80,26 +90,40 @@ def self.validate_and_save(l1_block_number, l2_block_hashes) | |
|
|
||
| # Timing info | ||
| validation_duration_ms: ((Time.current - start_time) * 1000).round(2) | ||
| } | ||
| vr.validated_at = Time.current | ||
| end | ||
| }, | ||
| validated_at: Time.current | ||
| ) | ||
|
|
||
| validation_result.save! | ||
|
|
||
| # Log the result | ||
| validation_result.log_summary | ||
|
|
||
| validation_result | ||
| rescue BlockValidator::TransientValidationError => e | ||
| # Don't persist transient errors - let ValidationJob handle retries | ||
| Rails.logger.debug "ValidationResult: Transient error for block #{l1_block_number}: #{e.message}" | ||
| raise e | ||
| rescue => e | ||
| Rails.logger.error "ValidationResult: Exception validating block #{l1_block_number}: #{e.message}" | ||
|
|
||
| # Record the validation error | ||
| error_result = create_or_find_by(l1_block: l1_block_number) do |vr| | ||
| vr.success = false | ||
| vr.error_details = [e.message] | ||
| vr.validation_stats = { exception: true, exception_class: e.class.name } | ||
| vr.validated_at = Time.current | ||
| end | ||
| # Only persist non-transient validation errors - idempotent for re-runs | ||
| validation_result = find_or_initialize_by(l1_block: l1_block_number) | ||
|
|
||
| validation_result.assign_attributes( | ||
| success: false, | ||
| error_details: [e.message], | ||
| validation_stats: { | ||
| exception: true, | ||
| exception_class: e.class.name, | ||
| validated_at: Time.current | ||
| }, | ||
| validated_at: Time.current | ||
| ) | ||
|
|
||
| error_result.log_summary | ||
| validation_result.save! | ||
|
|
||
| validation_result.log_summary | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Race Condition in Concurrent Validation JobsThe |
||
| raise e | ||
| end | ||
| end | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -8,6 +8,8 @@ class BlockNotReadyToImportError < StandardError; end | |||||
| class ReorgDetectedError < StandardError; end | ||||||
| # Raised when validation failure is detected (should stop system permanently) | ||||||
| class ValidationFailureError < StandardError; end | ||||||
| # Raised when validation is too far behind (should wait and retry) | ||||||
| class ValidationStalledError < StandardError; end | ||||||
|
|
||||||
| attr_accessor :ethscriptions_block_cache, :ethereum_client, :eth_block_cache, :geth_driver, :prefetcher | ||||||
|
|
||||||
|
|
@@ -196,13 +198,20 @@ def import_blocks_until_done | |||||
|
|
||||||
| begin | ||||||
| loop do | ||||||
| # Check for validation failures before importing | ||||||
| if validation_failure_detected? | ||||||
| # Check for validation failures | ||||||
| if real_validation_failure_detected? | ||||||
| failed_block = get_validation_failure_block | ||||||
| logger.error "Import stopped due to validation failure at block #{failed_block}" | ||||||
| raise ValidationFailureError.new("Validation failure detected at block #{failed_block}") | ||||||
| end | ||||||
|
|
||||||
| # Check if validation is stalled | ||||||
| if validation_stalled? | ||||||
| current_position = current_max_eth_block_number | ||||||
| logger.warn "Import paused - validation is behind (current: #{current_position})" | ||||||
| raise ValidationStalledError.new("Validation stalled - waiting for validation to catch up") | ||||||
| end | ||||||
|
|
||||||
| block_number = next_block_to_import | ||||||
|
|
||||||
| if block_number.nil? | ||||||
|
|
@@ -409,18 +418,42 @@ def geth_driver | |||||
| @geth_driver | ||||||
| end | ||||||
|
|
||||||
| def validation_failure_detected? | ||||||
| # Only consider failures BEHIND current import position as critical | ||||||
| def get_validation_failure_block | ||||||
| # Get the earliest failed block that's behind current import (for real failures) | ||||||
| current_position = current_max_eth_block_number | ||||||
| ValidationResult.failed.where('l1_block <= ?', current_position).order(:l1_block).first&.l1_block | ||||||
| end | ||||||
|
|
||||||
| def real_validation_failure_detected? | ||||||
| # Only consider real validation failures BEHIND current import position as critical | ||||||
| current_position = current_max_eth_block_number | ||||||
| ValidationResult.failed.where('l1_block <= ?', current_position).exists? | ||||||
| end | ||||||
|
|
||||||
| def get_validation_failure_block | ||||||
| # Get the earliest failed block that's behind current import | ||||||
| def validation_stalled? | ||||||
| return false unless ENV.fetch('VALIDATION_ENABLED').casecmp?('true') | ||||||
|
||||||
| return false unless ENV.fetch('VALIDATION_ENABLED').casecmp?('true') | |
| return false unless ENV.fetch('VALIDATION_ENABLED', 'false').casecmp?('true') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
validated_atfield is being set in both the validation_stats hash and as a separate attribute. Remove the duplicate entry from the validation_stats hash to avoid confusion.