[Playground] [Backend] Update playground cache component to increase fault tolerance #23461
Conversation
…object requests correctly
playground/backend/README.md
Outdated
| - `SDK_CONFIG` - is the sdk configuration file path, e.g. default example for corresponding sdk. It will be saved to cloud datastore during application startup (default value = `../sdks.yaml`) | ||
| - `DATASTORE_EMULATOR_HOST` - is the datastore emulator address. If it is given in the environment, the application will connect to the datastore emulator. | ||
| - `PROPERTY_PATH` - is the application properties path (default value = `.`) | ||
| - `CACHE_REQUEST_TIMEOUT` - is the timeout to request data from cache (default value = `30 sec`) |
There was a problem hiding this comment.
It should not be longer than HTTP timeout, we need to check these values on startup
There was a problem hiding this comment.
Make sense, but there is no timeout for requests to retrieve data about precompiled objects. We have timeout only for code processing when a pipeline is created
| if err != nil { | ||
| logger.Errorf("error during getting the sdk catalog from the cache, err: %s", err.Error()) | ||
| sdks, err = cp.db.GetSDKs(ctx) | ||
| func (cp *CacheComponent) GetSdkCatalogFromCacheOrDatastore(ctx context.Context, cacheRequestTimeout time.Duration) ([]*entity.SDKEntity, error) { |
There was a problem hiding this comment.
let's make timeout a parameter of a CacheComponent, what you think?
There was a problem hiding this comment.
It's not flexible to configure this parameter this way, i'd like to stay it as an env variable
| sdksCh := make(chan []*entity.SDKEntity, 1) | ||
| errCh := make(chan error, 1) | ||
| go func() { | ||
| sdks, err := cp.cache.GetSdkCatalog(ctx) |
There was a problem hiding this comment.
let's just do cache request with a reduced timeout:
ctx = context.WithTimeout(ctx, CACHE_TO)
no need for goroutines here
| return cat, nil | ||
| case e := <-errCh: | ||
| logger.Errorf("error during getting the catalog from the cache, err: %s", e.Error()) | ||
| return cp.getCatalog(ctx, cacheRequestTimeout) |
There was a problem hiding this comment.
timeout is a part of a context
https://pkg.go.dev/context#WithDeadline
| examples_ids_before_updating = self._get_all_examples(sdk) | ||
|
|
||
| # loop through every example to save them to the Cloud Datastore | ||
| with self._datastore_client.transaction(): |
There was a problem hiding this comment.
I removed the transaction here because it caused the error related to datastore restriction during deleting an entity group.
Error:
google.api_core.exceptions.InvalidArgument: 400 operating on too many entity groups in a single transaction.
Doc:
|
R: @pabloem |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control |
# Conflicts: # playground/infrastructure/datastore_client.py
|
lgtm |
…fault tolerance (apache#23461) * [Playground] [Backend] updated cache component to handle precompiled object requests correctly * [Playground] [Backend] added context with timeout instead of goroutine * [Playground] [Backend] edited cache request timeout variable * [Playground] [Backend] refactoring after review * [Playground] [Backend] changed default value in readme * [Playground] [Backend] removed transaction for ci cd datastore client
3 participants
fixes #23458
@eantyshev
@olehborysevych
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
R: @username).addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI.