Skip to content
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

public archive sdk #2426

Merged
merged 19 commits into from
Jan 8, 2025
Merged

public archive sdk #2426

merged 19 commits into from
Jan 8, 2025

Conversation

arturrez
Copy link
Collaborator

@arturrez arturrez commented Dec 11, 2024

Why this should be merged

resolves #2425
it provides sdk pkg to access public archive data for fuji to speedup avalanchego bootstrap if connected to fuji
current timing

1 node 27.877
2 nodes 31.086
7 nodes 1:25.34

How this works

only fuji is supported for now.

sdk provides a functionality to Download public archive data into tmp file. This data can be un-tar into destination folder.
this sdk is used inside of the CLI. In case of the error all downloaded data is cleaned and bootstrap continues on like nothing happened.

How this was tested

./bin/avalanche node local start  --num-nodes 1 --fuji --avalanchego-path ~/src/github.com/ava-labs/avalanchego/build/avalanchego fuji1
./bin/avalanche node local start  --num-nodes 2 --fuji --avalanchego-path ~/src/github.com/ava-labs/avalanchego/build/avalanchego fuji2

How is this documented

n/a

@arturrez arturrez requested a review from a team as a code owner December 11, 2024 00:26
@arturrez arturrez changed the title Achilles sdk public archive sdk Dec 11, 2024
@@ -490,6 +498,9 @@ func UpsizeLocalNode(
nodeConfig = map[string]interface{}{}
}
nodeConfig[config.NetworkAllowPrivateIPsKey] = true
if network.Kind == models.Fuji {
nodeConfig[config.IndexEnabledKey] = false // disable index for Fuji
Copy link
Collaborator

Choose a reason for hiding this comment

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

what does it mean if this flag is disabled?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

dataset for avalanchego created with index and without are not interchangeable

}

// NewDownloader returns a new Downloader
// network: the network to download from ( fuji or mainnet). todo: add mainnet support
Copy link
Collaborator

Choose a reason for hiding this comment

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

lets remove this mainnet support part

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

k

}, nil
}
default:
return Downloader{}, fmt.Errorf("unsupported network ID: %d", network.ID)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can just print currently public archive is only supported on fuji

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added

return nil
}

func (d Downloader) UnpackTo(targetDir string) error {
Copy link
Collaborator

@sukantoraymond sukantoraymond Jan 7, 2025

Choose a reason for hiding this comment

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

Is there no library for this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

we also have code for this

Comment on lines +178 to +211
switch header.Typeflag {
case tar.TypeDir:
d.logger.Debug("Creating directory", zap.String("path", targetPath))
if err := os.MkdirAll(targetPath, os.FileMode(header.Mode)); err != nil {
d.logger.Error("Failed to create directory", zap.Error(err))
return fmt.Errorf("failed to create directory: %w", err)
}
case tar.TypeReg:
d.logger.Debug("Ensure parent directory exists for ", zap.String("path", targetPath))
if err := os.MkdirAll(filepath.Dir(targetPath), os.FileMode(0o755)); err != nil {
d.logger.Error("Failed to create parent directory for file", zap.Error(err))
return fmt.Errorf("failed to create parent directory for file: %w", err)
}
d.logger.Debug("Creating file", zap.String("path", targetPath))
outFile, err := os.Create(targetPath)
if err != nil {
d.logger.Error("Failed to create file", zap.Error(err))
return fmt.Errorf("failed to create file: %w", err)
}
defer outFile.Close()
copied, err := io.CopyN(outFile, tarReader, header.Size)
if err != nil {
d.logger.Error("Failed to write file", zap.Error(err))
return fmt.Errorf("failed to write file: %w", err)
}
if copied < header.Size {
d.logger.Error("Incomplete file write", zap.String("path", targetPath))
return fmt.Errorf("incomplete file write for %s", targetPath)
}
extractedSize += header.Size
d.logger.Debug("Written bytes", zap.Int64("bytes", extractedSize))
default:
d.logger.Debug("Skipping file", zap.String("path", targetPath))
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

might be better for us to use library

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i didn't find one that I liked

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am open for the suggestions

if err := publicArcDownloader.Download(); err != nil {
return fmt.Errorf("failed to download public archive: %w", err)
}
// defer publicArcDownloader.CleanUp()
Copy link
Collaborator

Choose a reason for hiding this comment

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

does this need to be deleted

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good catch. it should be enabled and was commented out for the debug


wg := sync.WaitGroup{}
mu := sync.Mutex{}
var firstErr error
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe we should rename this err var

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

err is used in

if firstErr == nil {
					firstErr = fmt.Errorf("failed to unpack public archive: %w", err)

so it should be some other name

ux.Logger.Info("unpacking public archive to %s", target)

// Skip if target already exists
if _, err := os.Stat(target); err == nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we override data instead of skip?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no we should skip as data is already there

sdk/publicarchive/helpers.go Outdated Show resolved Hide resolved
sdk/publicarchive/helpers.go Outdated Show resolved Hide resolved
"github.com/cavaliergopher/grab/v3"
"go.uber.org/zap"

sdkConstants "github.com/ava-labs/avalanche-cli/sdk/constants"
Copy link
Collaborator

Choose a reason for hiding this comment

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

inside sdk I shall call this one constants, and the other one cliConstants

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

agree

// logLevel: the log level
func NewDownloader(
network network.Network,
logLevel logging.Level,
Copy link
Collaborator

Choose a reason for hiding this comment

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

can it receive a logger instead of log level

Copy link
Collaborator

Choose a reason for hiding this comment

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

that is to have full control on the logs

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

agree

return nil
}

func (d Downloader) UnpackTo(targetDir string) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

we also have code for this

@@ -10,5 +10,6 @@ const (
APIRequestLargeTimeout = 2 * time.Minute

// node
WriteReadUserOnlyPerms = 0o600
WriteReadUserOnlyPerms = 0o600
WriteReadUserOnlyDirPerms = 0o700
Copy link
Collaborator

Choose a reason for hiding this comment

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

UserOnlyReadWriteExecPerms ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

k

return nil
}

func CleanUpClusterNodeData(rootDir string, nodesNames []string) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe this is not needed to be public

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

agree. fixed

@@ -555,3 +560,75 @@ func GetNodeData(endpoint string) (
"0x" + hex.EncodeToString(proofOfPossession.ProofOfPossession[:]),
nil
}

func SeedClusterData(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can it have another name. Something related to downloading blockchain db for the cluster

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

changed to DownloadPublicArchive

Copy link
Collaborator

@felipemadero felipemadero left a comment

Choose a reason for hiding this comment

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

LGTM

@arturrez arturrez merged commit 3e3ccce into main Jan 8, 2025
37 checks passed
@arturrez arturrez deleted the achilles-sdk branch January 8, 2025 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

speedup fuji L1 creation on local using achilles
3 participants