Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

Fix bootstrap without known validators #25293

Merged
merged 1 commit into from
May 18, 2022
Merged

Fix bootstrap without known validators #25293

merged 1 commit into from
May 18, 2022

Conversation

diman-io
Copy link
Contributor

Problem

Validator with --incremental-snapshots can't start to download snapshot if no one --known-validator set (not depended from --only-known-rpc)

Summary of Changes

Do not match snapshot hashes with snapshot hashes of known validators if no one is set.

Fixes #23927

@mergify mergify bot added the community Community contribution label May 17, 2022
@mergify mergify bot requested a review from a team May 17, 2022 16:39
@diman-io diman-io requested a review from brooksprumo May 17, 2022 16:39
@diman-io diman-io marked this pull request as ready for review May 17, 2022 16:39
@diman-io diman-io removed the request for review from a team May 17, 2022 16:39
brooksprumo
brooksprumo reviewed May 17, 2022
View reviewed changes
Copy link
Contributor

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

This looks good to me. How has it been tested?

@diman-io
Copy link
Contributor Author

diman-io commented May 17, 2022 •
edited
Loading

@brooksprumo You are right. It so little that I just compile.
But after your question I tried to start validator and it looks like this commit is not enough.
I'm looking now.
Can say that backporting will be with manual conflict resolving.

UPD. My bad 🙊🙊🙊 Ran the original build by mistake. Everything is fine

@diman-io
Copy link
Contributor Author

diman-io commented May 17, 2022 •
edited
Loading

Checked:

  • master
  • 1.10.14
  • 1.9.22

Everyone starts downloading a snapshot without known validators.

@diman-io diman-io requested a review from brooksprumo May 17, 2022 18:26
@brooksprumo
Copy link
Contributor

Checked:

  • master
  • 1.10.14
  • 1.9.22

Everyone starts downloading a snapshot without known validators.

Excellent, glad to hear! Can you confirm that all three versions tested had incremental snapshots enabled? I know master and v1.10 have incremental snapshots enabled by default, but v1.9 does not.

@diman-io
Copy link
Contributor Author

Can you confirm that all three versions tested had incremental snapshots enabled? I know master and v1.10 have incremental snapshots enabled by default, but v1.9 does not.

Yes, all three with --incremental-snapshots

brooksprumo
brooksprumo previously approved these changes May 17, 2022
View reviewed changes
Copy link
Contributor

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Will need to rebase once a fix is in for the ci/stable job. Either PR #25295 or #25298.

@steviez
Copy link
Contributor

steviez commented May 17, 2022 •
edited
Loading

Will need to rebase once a fix is in for the ci/stable job. Either PR #25295 or #25298.

#25298 ended up being the fix and is in, so go ahead and rebase on tip of master for new CI run

@diman-io diman-io force-pushed the no-known-validators branch from bc17c76 to 4dd16ed Compare May 18, 2022 08:51
@mergify mergify bot dismissed brooksprumo’s stale review May 18, 2022 08:54

Pull request has been modified.

@codecov
Copy link

codecov bot commented May 18, 2022

Codecov Report

Merging #25293 (4dd16ed) into master (69a0ff9) will increase coverage by 0.0%.
The diff coverage is 66.8%.

@@            Coverage Diff            @@
##           master   #25293     +/-   ##
=========================================
  Coverage    82.0%    82.1%             
=========================================
  Files         598      614     +16     
  Lines      165882   169315   +3433     
=========================================
+ Hits       136125   139016   +2891     
- Misses      29757    30299    +542

@brooksprumo brooksprumo merged commit 2412276 into solana-labs:master May 18, 2022
mergify bot pushed a commit that referenced this pull request May 18, 2022
@diman-io @mergify
fix bootstrap without known validators (#25293)
b8a539f 
(cherry picked from commit 2412276)

# Conflicts:
#	validator/src/bootstrap.rs
@mergify mergify bot mentioned this pull request May 18, 2022
Merged
mergify bot pushed a commit that referenced this pull request May 18, 2022
@diman-io @mergify
fix bootstrap without known validators (#25293)
57e94b3 
(cherry picked from commit 2412276)

# Conflicts:
#	validator/src/bootstrap.rs
@mergify mergify bot mentioned this pull request May 18, 2022
Merged
mergify bot added a commit that referenced this pull request May 18, 2022
@mergify @diman-io @brooksprumo
Fix bootstrap without known validators (backport #25293) (#25319)
a690c36 
* fix bootstrap without known validators (#25293)

(cherry picked from commit 2412276)

# Conflicts:
#	validator/src/bootstrap.rs

* pr: fix merge conflicts

Co-authored-by: DimAn <diman@diman.io>
Co-authored-by: Brooks Prumo <brooks@solana.com>
mergify bot added a commit that referenced this pull request May 18, 2022
@mergify @diman-io @brooksprumo
Fix bootstrap without known validators (backport #25293) (#25318)
a227f9a 
* fix bootstrap without known validators (#25293)

(cherry picked from commit 2412276)

# Conflicts:
#	validator/src/bootstrap.rs

* pr: fix merge conflicts

* pr: checks

Co-authored-by: DimAn <diman@diman.io>
Co-authored-by: Brooks Prumo <brooks@solana.com>
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Jun 29, 2022
@diman-io @jeffwashington
fix bootstrap without known validators (solana-labs#25293)
a6955a9
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Jun 29, 2022
@diman-io @jeffwashington
fix bootstrap without known validators (solana-labs#25293)
1620dd3
@diman-io diman-io deleted the no-known-validators branch May 13, 2023 21:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@brooksprumo brooksprumo brooksprumo left review comments

Assignees
No one assigned
Labels
community Community contribution
Projects
None yet
Milestone
No milestone
3 participants
@diman-io @brooksprumo @steviez