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

feat(server): multi region blob support #3653

Merged
merged 11 commits into from
Dec 10, 2024

Conversation

fabis94
Copy link
Contributor

@fabis94 fabis94 commented Dec 6, 2024

No description provided.

Copy link

linear bot commented Dec 6, 2024

@@ -144,18 +93,38 @@ describe('Blobs integration @blobstorage', () => {
email: '[email protected]',
password: 'bubblesAreMyBlobs'
}
const workspace = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

with only these changes this entire test suite now runs in both non-multiregion and multiregion mode, and actually properly stores data (both db rows & blobs) into region storage

Copy link
Contributor

github-actions bot commented Dec 6, 2024

📸 Preview service has generated an image.

.circleci/config.yml Outdated Show resolved Hide resolved
.circleci/config.yml Outdated Show resolved Hide resolved
Copy link
Contributor

github-actions bot commented Dec 6, 2024

📸 Preview service has generated an image.

Copy link
Contributor

github-actions bot commented Dec 6, 2024

📸 Preview service has generated an image.

Copy link
Contributor

github-actions bot commented Dec 6, 2024

📸 Preview service has generated an image.

Copy link
Contributor

github-actions bot commented Dec 6, 2024

📸 Preview service has generated an image.

Copy link
Contributor

github-actions bot commented Dec 6, 2024

📸 Preview service has generated an image.

Copy link
Contributor

github-actions bot commented Dec 6, 2024

📸 Preview service has generated an image.

Copy link
Contributor

github-actions bot commented Dec 6, 2024

📸 Preview service has generated an image.

await ensureStorageAccess()
const storage = getMainObjectStorage()
const ensureStorageAccess = ensureStorageAccessFactory({ storage })
await ensureStorageAccess({
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't seem to be ensuring that we can access all the region storage?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That happens elsewhere (initializeRegisteredRegionClients())

Copy link
Contributor Author

Choose a reason for hiding this comment

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

or is this a separate procedure from the one in the module's index.ts? why do we need to even invoke this if this gets validated on module init?

Copy link
Contributor

Choose a reason for hiding this comment

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

multiregion/index.ts -> initializeRegisteredRegionClients() in multiregion/utils/blobStorageSelector -> initializeRegion -> ensureStorageAccessFactory. (This tests each additional regions, but ignores the primary region from multi-region config file.)

blobstorage/index.ts -> ensureStorageAccessFactory (This tests the 'main' storage, which is configured via the original environment variables and not from the multi-region config file)

We can ignore my comment at the top of this thread. And flipping from the original environment variables to the primary block of the multi-region config file can be done separately from this PR to align with the postgres database.

packages/shared/src/environment/multiRegionConfig.ts Outdated Show resolved Hide resolved
Copy link
Contributor

github-actions bot commented Dec 9, 2024

📸 Preview service has generated an image.

@iainsproat iainsproat marked this pull request as ready for review December 9, 2024 16:13
@iainsproat iainsproat merged commit 8d0cbad into main Dec 10, 2024
43 of 44 checks passed
@iainsproat iainsproat deleted the fabians/web-2296-multi-region-blobs branch December 10, 2024 09:05
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