Skip to content

fix: accept Graphsync measurements without HEAD status #498

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

Merged
merged 1 commit into from
Mar 11, 2025

Conversation

bajtos
Copy link
Member

@bajtos bajtos commented Mar 10, 2025

Fix the regression introduced by e2a458b (#475)

  • Graphsync retrievals don't test HEAD request, see CheckerNetwork/spark-checker@3f8edc5
  • As a result, measurements for Graphsync retrievals have head_status_code set to undefined or null.
  • The validation performed during the preprocessing step discarded such measurements as invalid.

This pull requests relaxes the validation during preprocessing to assert numeric head_status_code for HTTP retrievals only.

Fix the regression introduced by e2a458b (#475)

Signed-off-by: Miroslav Bajtoš <[email protected]>
@bajtos bajtos merged commit b66a82c into main Mar 11, 2025
6 checks passed
@bajtos bajtos deleted the fix-graphsync-measurements branch March 11, 2025 05:41
@github-project-automation github-project-automation bot moved this to ✅ done in Space Meridian Mar 11, 2025
@juliangruber
Copy link
Member

Thank you for fixing this! 🤗

bajtos added a commit to CheckerNetwork/spark-api that referenced this pull request Mar 11, 2025
We have recently introduced a new requirement - when an HTTP retrieval
succeeded, the checker is required to test HEAD retrieval request too.
The preprocessing step in spark-evaluate discards measurements missing
`head_status_code`.

Because a large part of the network is running an older version of
Spark Checker, measurements from these nodes are rejected as invalid.
See CheckerNetwork/spark-evaluate#498.

This patch is moving the validation step earlier to the pipeline
by changing the minimum required Spark Checker version to 1.17.0.

Checker nodes running an older version will start receiving a helpful
"OUTDATED CLIENT" error response.

Signed-off-by: Miroslav Bajtoš <[email protected]>
@bajtos
Copy link
Member Author

bajtos commented Mar 11, 2025

After deploying this fix, Spark RSR jumped back to ~20%, where it was before Feb 28th.

Screenshot 2025-03-11 at 12 47 36

bajtos added a commit to CheckerNetwork/spark-api that referenced this pull request Mar 12, 2025
We have recently introduced a new requirement - when an HTTP retrieval
succeeded, the checker is required to test HEAD retrieval request too.
The preprocessing step in spark-evaluate discards measurements missing
`head_status_code`.

Because a large part of the network is running an older version of
Spark Checker, measurements from these nodes are rejected as invalid.
See CheckerNetwork/spark-evaluate#498.

This patch is moving the validation step earlier to the pipeline
by changing the minimum required Spark Checker version to 1.17.0.

Checker nodes running an older version will start receiving a helpful
"OUTDATED CLIENT" error response.

Signed-off-by: Miroslav Bajtoš <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ done
Development

Successfully merging this pull request may close these issues.

3 participants