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

Fix INVALID_SCHEMA_DB objectstats ERRORS #7373

Merged

Conversation

tangledbytes
Copy link
Member

Explain the changes

This PR fixes INVALID_SCHEMA_DB objectstats ERRORS that we encounter in our endpoints.

The root cause for the issue was due to the fact that postgres (by default/without COALASCE) does not create nested fields. This problem is being fixed in this PR via a hack field in findOneAndUpdate's options parameter called upsert_options which creates the necessarily fields if the entry is new.

Other solutions to the problem were also explored like:

  1. Changing the upstream dependency to handle upserts better. Rejected due to potentially slow dev cycle.
  2. Update noobaa internal methods to be smarter with handling upserts. Rejected due to fear of regression on other part of codebase.
  3. Using 2 findOneAndUpdate. Works but doesn't works both theoretically and practically. Too many races. Rejected.

Although the changes are kind of postgres specific and tested on postgres exclusively, I fully expect this to function well in mongo as well.

  • Doc added/updated
  • Tests added

Comment on lines +77 to +84
// Ineffective and not needed in mongo
//
// This is a postgres hack, this allows us to
// initialize the counters if the fields are missing
upsert_fields: {
s3_usage_info: usage,
s3_errors_info: errors
}
Copy link
Member

Choose a reason for hiding this comment

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

Hey @dannyzaken - PTAL
I remember you said there is something to fix for upsert - is that in the direction you where thinking?

Copy link
Contributor

Choose a reason for hiding this comment

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

This fix looks good. In general, I think we should get rid of upserts altogether since the way we implement it in Postgres is too complex. This will require changing our stats stores that still use it

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, but we might still need upsert or at least an atomic find + insert (a transaction based wrapper maybe)?

Of course not as part of this PR, rather a separate PR addressing PG upserts.

@tangledbytes tangledbytes force-pushed the utkarsh-pro/fix/boreas-mvp-19 branch from 2464dee to ea12224 Compare July 6, 2023 10:02
Signed-off-by: Utkarsh Srivastava <srivastavautkarsh8097@gmail.com>
@tangledbytes tangledbytes force-pushed the utkarsh-pro/fix/boreas-mvp-19 branch from ea12224 to 872ca7d Compare July 6, 2023 10:40
@tangledbytes tangledbytes merged commit e34e92e into noobaa:master Jul 6, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants