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

Mirror feature #586

Merged
merged 1 commit into from
Jan 25, 2023
Merged

Mirror feature #586

merged 1 commit into from
Jan 25, 2023

Conversation

jeanp413
Copy link
Contributor

@jeanp413 jeanp413 commented Nov 8, 2022

Fixes #505

In the mirror mode OpenVSX registry allows only read-only access to metadata and in the background mirrors metadata from the upstream, i.e. open-vsx.org. Upstream storage is still used to fetch actual content.

Search queries are performed against the upstream first to ensure that latest extension versions are available. But if requests take more than 2secs then they are interrupted and the mirror falls back to local DB. It ensures that the mirror is responsive even if upstream is not, additionally preventing heavy request to run on upstream. Metadata for concrete versions are first looked up from the local mirror and only from the upstream.

ovsx_mirror_versions metric allows to observe progress of mirroring individual extension versions. Detailed logs are available for troubleshooting on jobrun dashboard for the mirror data job. Exception during mirroring logged as well to the registry logs.

The mirror mode was load tested and tuned to sustain 15x load which is generated by Gitpod, i.e 250 requests per second to search for random extensions or resolve its versions from VS Code. The test was performed in different scenarios when upstream is up, down or slow. It was enough to run one instance with 4Gb of RAM and 3 CPUs to sustain it and at the same time mirror all extension versions within 18 hours. This PR adds observability metrics which were necessary to track down performance issues and monitor the app during load testing.

How to test

TODO:...

  • Smoke test usual mode.
  • How to enable the mirror mode.
  • mirror job logs
  • observability

@akosyakov akosyakov mentioned this pull request Nov 8, 2022
@amvanbaren amvanbaren marked this pull request as ready for review November 28, 2022 14:46
@filiptronicek
Copy link
Member

I'm working on addressing issues that were noted in the review. This is why some discussions are marked as resolved; the fixing commit is not yet pushed.

@filiptronicek filiptronicek force-pushed the mirror-feature branch 3 times, most recently from 9fbc9ad to 98e50a5 Compare December 12, 2022 17:49
@filiptronicek filiptronicek force-pushed the mirror-feature branch 4 times, most recently from aab1f24 to 9b195d9 Compare December 13, 2022 15:55
@filiptronicek
Copy link
Member

filiptronicek commented Dec 13, 2022

I have compared 7d039aa and the changes made in this branch and reverted hopefully the remaining changes that should not be included here – hence 9b195d9.

Also reverted the elastic replicas disabling in 5907b37.

Left unsquashed for now so that it's hopefully easier understood / easier to work with if we want to revert or similar. Will squash after everything compiles and everyone is good with the changes.

Closed and re-opened by a keyboard combo accident 😄

@filiptronicek
Copy link
Member

@amvanbaren could you please take another look? Me and @akosyakov settled on refraining from big structural refactors, it would be great to bring this to eclipse:main soon so that we can iterate on it there via subsequent PRs 🙏

@akosyakov
Copy link
Member

@filiptronicek I think we need to rebase and squash commits again, please preserve authorship and co contributors.

@filiptronicek
Copy link
Member

Rebased and squashed ✅

cc @akosyakov

@amvanbaren
Copy link
Contributor

@filiptronicek I'll add fixes for the remaining open conversations.

@amvanbaren amvanbaren self-assigned this Jan 5, 2023
@filiptronicek
Copy link
Member

Thank you, I have tried implementing the Conditional Interceptors, but I had a pretty hard time, will see afterwards to see how you did it!

@amvanbaren amvanbaren force-pushed the mirror-feature branch 2 times, most recently from e29bd3b to c69991d Compare January 14, 2023 13:36
@amvanbaren
Copy link
Contributor

@jeanp413 Can you test mirror mode performance with the new changes?

@jeanp413
Copy link
Contributor Author

I will do some testing today.

But we had a chat with @akosyakov and we agreed that we are confident in general this latest changes doesn't affect production reliability we can just merge it, then we can deploy internally in staging and if there's some fixes needed we can create more PRs

cc @laushinka just so you have this in your radar

@jeanp413
Copy link
Contributor Author

I'm seeing this error, any idea @amvanbaren ?

2023-01-21 21:41:19.990  WARN 18819 --- [  restartedMain] ConfigServletWebServerApplicationContext : Exception encountered during context initialization - cancelling refresh attempt: org.springframework.beans.factory.UnsatisfiedDependencyException: Error creating bean with name 'adminAPI': Unsatisfied dependency expressed through field 'admins'; nested exception is org.springframework.beans.factory.UnsatisfiedDependencyException: Error creating bean with name 'adminService': Unsatisfied dependency expressed through field 'eclipse'; nested exception is org.springframework.beans.factory.UnsatisfiedDependencyException: Error creating bean with name 'eclipseService': Unsatisfied dependency expressed through field 'tokens'; nested exception is org.springframework.beans.factory.UnsatisfiedDependencyException: Error creating bean with name 'tokenService': Unsatisfied dependency expressed through field 'clientRegistrationRepository'; nested exception is org.springframework.beans.factory.UnsatisfiedDependencyException: Error creating bean with name 'clientRegistrationRepository' defined in class path resource [org/springframework/boot/autoconfigure/security/oauth2/client/servlet/OAuth2ClientRegistrationRepositoryConfiguration.class]: Unsatisfied dependency expressed through method 'clientRegistrationRepository' parameter 0; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'spring.security.oauth2.client-org.springframework.boot.autoconfigure.security.oauth2.client.OAuth2ClientProperties': Invocation of init method failed; nested exception is java.lang.IllegalStateException: Client id must not be empty.

@amvanbaren
Copy link
Contributor

I'm seeing this error, any idea @amvanbaren ?

spring.security.oauth2.client.registration.github.client-id isn't defined.

@jeanp413
Copy link
Contributor Author

jeanp413 commented Jan 24, 2023

@amvanbaren I tested it yesterday and was working fine, performance seems slightly better 👍, is something else missing so we can merge it?

@amvanbaren amvanbaren force-pushed the mirror-feature branch 2 times, most recently from e0a2302 to f257bc8 Compare January 25, 2023 14:59
Co-authored-by: Jean Pierre <jeanpierre@gitpod.io>
Co-authored-by: Pudong <tianshi8650@gmail.com>
Co-authored-by: Huiwen <huiwen@gitpod.io>
Co-authored-by: akosyakov <anton@gitpod.io>
@amvanbaren amvanbaren merged commit 02a7bb5 into eclipse:master Jan 25, 2023
@jeanp413
Copy link
Contributor Author

Thanks @amvanbaren 🎉

@jeanp413 jeanp413 deleted the mirror-feature branch January 25, 2023 19:45
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.

Mirror mode
4 participants