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

avoid attempting to migrate old configs #17004

Merged
merged 1 commit into from
Apr 21, 2023

Conversation

harshavardhana
Copy link
Member

@harshavardhana harshavardhana commented Apr 10, 2023

Description

avoid attempting to migrate old configs

Motivation and Context

instead of migration, move them in memory to a
local representation of new config.

In all other cases re-initialize if config's
missing.

How to test this PR?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Optimization (provides speedup with no functional changes)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • Fixes a regression (If yes, please add commit-id or PR # here)
  • Unit tests added/updated
  • Internal documentation updated
  • Create a documentation update request here

@harshavardhana harshavardhana marked this pull request as ready for review April 11, 2023 19:01
@harshavardhana harshavardhana changed the title [DRAFT] avoid attempt to migrate old configs avoid attempt to migrate old configs Apr 11, 2023
@harshavardhana harshavardhana changed the title avoid attempt to migrate old configs avoid attempting to migrate old configs Apr 11, 2023
Copy link
Contributor

@klauspost klauspost left a comment

Choose a reason for hiding this comment

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

LGTM. I presume you tested as much as possible.

@vadmeste
Copy link
Member

I think maybe we should start with adding MinIO version in format.json. It may be useful to prevent direct upgrade from very old versions when we do very old code clean up.

Copy link
Member

@vadmeste vadmeste left a comment

Choose a reason for hiding this comment

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

@harshavardhana LGTM, but maybe if we want to just drop support of older config versions to clean up existing code, we can error out after testing on config version (<=33) and tell that the user needs to upgrade to 'RELEASE.2023-04-07T05-28-58Z' first.

@harshavardhana
Copy link
Member Author

@harshavardhana LGTM, but maybe if we want to just drop support of older config versions to clean up existing code, we can error out after testing on config version (<=33) and tell that the user needs to upgrade to 'RELEASE.2023-04-07T05-28-58Z' first.

We can but it's a bunch of stuff to document @vadmeste and confusing. I would like to avoid that completely.

@klauspost I did I will test more, there is one more change missing here. I am removing migration code for credentials based encryption to KMS based.

@harshavardhana
Copy link
Member Author

re-did the decryption migration as well, we will never need to migrate any old content, we simply read them always as their original entity.

But let the WRITEs make sure that we write back in the new format, so we do not need to hold
MinIO startup migrating content.

instead of migration, move them in-memory to a
local representation of new config.

In all other cases re-initialize if config's
missing.
@harshavardhana harshavardhana merged commit 477230c into minio:master Apr 21, 2023
16 checks passed
@harshavardhana harshavardhana deleted the do-not-migrate branch April 21, 2023 20:56
harshavardhana added a commit to harshavardhana/minio that referenced this pull request Apr 24, 2023
the amount of listing calls this can start can easily overwhelm the
server and add to latencies upon startup on a large setup. With the
change in the PR minio#17004 - we have no reason to heal or migrate
old formats.

We can always read and let them be overwritten in time.
harshavardhana added a commit to harshavardhana/minio that referenced this pull request Apr 25, 2023
The number of listing calls this can start can easily overwhelm
the server and add to latencies upon startup on a large setup.
With the change in PR minio#17004 - we have no reason to heal or
migrate old formats.

We can always read and let them be overwritten in time.
harshavardhana added a commit to harshavardhana/minio that referenced this pull request Apr 25, 2023
The number of listing calls this can start can easily overwhelm
the server and add to latencies upon startup on a large setup.
With the change in PR minio#17004 - we have no reason to heal or
migrate old formats.

We can always read and let them be overwritten in time.
harshavardhana added a commit to harshavardhana/minio that referenced this pull request Apr 26, 2023
The number of listing calls this can start can easily overwhelm
the server and add to latencies upon startup on a large setup.
With the change in PR minio#17004 - we have no reason to heal or
migrate old formats.

We can always read and let them be overwritten in time.
harshavardhana added a commit to harshavardhana/minio that referenced this pull request Apr 26, 2023
The number of listing calls this can start can easily overwhelm
the server and add to latencies upon startup on a large setup.
With the change in PR minio#17004 - we have no reason to heal or
migrate old formats.

We can always read and let them be overwritten in time.
harshavardhana added a commit to harshavardhana/minio that referenced this pull request Apr 29, 2023
The number of listing calls this can start can easily overwhelm
the server and add to latencies upon startup on a large setup.
With the change in PR minio#17004 - we have no reason to heal or
migrate old formats.

We can always read and let them be overwritten in time.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants