Skip to content

Add input-reader module#2

Open
stephenctw wants to merge 1 commit intomainfrom
feature/safe-input-reader
Open

Add input-reader module#2
stephenctw wants to merge 1 commit intomainfrom
feature/safe-input-reader

Conversation

@stephenctw
Copy link
Collaborator

@stephenctw stephenctw commented Mar 1, 2026

Add safe-input-reader and related storage logic

@stephenctw stephenctw requested a review from GCdePaula March 1, 2026 12:45
@stephenctw stephenctw self-assigned this Mar 1, 2026
@stephenctw stephenctw force-pushed the feature/safe-input-reader branch 2 times, most recently from c95f733 to 16e155b Compare March 1, 2026 15:49
@stephenctw stephenctw force-pushed the feature/safe-input-reader branch from 16e155b to 42597c6 Compare March 1, 2026 15:54
Copy link
Collaborator

@GCdePaula GCdePaula left a comment

Choose a reason for hiding this comment

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

Nice work!

I’ve left two inline comments (range split + atomic cursor/input persistence).

There are two follow-ups I suggest we handle on top of my open branch:

  1. Input reader lifecycle
    Right now the reader thread is detached, so we don’t have proper shutdown/join/error propagation to main.
    In my open PR we already have a cleaner lifecycle pattern for background workers; I suggest we wire the reader into that same pattern.

  2. Config surface
    This PR introduces new env knobs.
    My branch has moved config parsing to clap, so we should add the new reader options there to keep configuration consistent.

Given that, my recommendation is:

  • merge my branch first,
  • then rebase this input-reader work on top,
  • and apply the two inline fixes plus the two follow-ups above.

Comment on lines +74 to +76
let blocks = 1 + end_block - start_block;
let half = blocks / 2;
let middle = start_block + half - 1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

We're not handling the base case. Moreover, the middle calculation can underflow.

I propose these changes:

if start_block >= end_block {
    // Cannot partition further; bubble original error.
    return Err(vec![e]);
}

let middle = start_block + (end_block - start_block) / 2;

Comment on lines +275 to +277
self.storage.append_safe_direct_inputs(&batch)?;
self.storage
.input_reader_set_last_processed_block(current)?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

These two operations have to be atomic; we need to perform them inside a single db transaction.

I'd create a single function in storage called something like append_safe_inputs_and_advance_cursor(inputs, new_last_processed_block) that performs both operations atomically.

@stephenctw
Copy link
Collaborator Author

Given that, my recommendation is:

  • merge my branch first,
  • then rebase this input-reader work on top,
  • and apply the two inline fixes plus the two follow-ups above.

Sounds good, let's merge your PR first, then adjust mine afterwards.

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.

2 participants