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

PM-832 Multi storage #26

Merged
merged 10 commits into from
Dec 13, 2023
Merged

PM-832 Multi storage #26

merged 10 commits into from
Dec 13, 2023

Conversation

piotr-iohk
Copy link
Collaborator

This pr adds ability to use many storage backends when running uptime service, i.e. one can configure AWS S3, AWS Keyspaces and Local filesystem at the same time or use any combination of the above (in particular AWS S3 and Keyspaces).

  • This is tested by integration test that was modified to account for the change.
  • Also manually tested behavior when the block exceeds 1MB. In such circumstance the block is saved on S3 and is not saved in Keyspaces with the following error message:
{"level":"warn",
"ts":"2023-12-12T12:23:32.825+0100",
"logger":"delegation backend",
"caller":"delegation_backend/aws_keyspaces.go:173",
"msg":"KeyspaceSave: Error saving block to Keyspaces: operation failed after 3 retries, returned error: The update would cause the row to exceed the maximum allowed size"}

For that the exponentialBackoff strategy was modified when inserting block/submission to keyspaces (3 retries, initial = 300ms, was: 10 retries , initial = 1s). I think new parameters better optimize for reasonable response time in case of transient issues still giving good error handling.

Copy link
Contributor

@Smorci Smorci 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!

Copy link
Member

@joaosreis joaosreis left a comment

Choose a reason for hiding this comment

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

LGTM! Just to double check, are those GPG files supposed to be included?

@piotr-iohk
Copy link
Collaborator Author

LGTM! Just to double check, are those GPG files supposed to be included?

Yes, they hold encoded config for integration test.

@piotr-iohk piotr-iohk merged commit 60504b0 into main Dec 13, 2023
2 checks passed
@piotr-iohk piotr-iohk deleted the multi-storage branch December 13, 2023 06:49
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.

3 participants