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

Adjust encoding of Azure block IDs #68957

Merged
merged 5 commits into from
Feb 15, 2021

Conversation

DaveCTurner
Copy link
Contributor

Today we represent block IDs sent to Azure using the URL-safe base-64
encoding. This makes sense: these IDs appear in URLs. It turns out that
Azure rejects this encoding for block IDs and instead demands that they
are represented using the regular, URL-unsafe, base-64 encoding instead,
then further wrapped in %-encoding to deal with the URL-unsafe
characters that inevitably result.

Relates #66489

Today we represent block IDs sent to Azure using the URL-safe base-64
encoding. This makes sense: these IDs appear in URLs. It turns out that
Azure rejects this encoding for block IDs and instead demands that they
are represented using the regular, URL-unsafe, base-64 encoding instead,
then further wrapped in %-encoding to deal with the URL-unsafe
characters that inevitably result.

Relates elastic#66489
@DaveCTurner DaveCTurner requested a review from fcofdez February 12, 2021 20:49
@elasticmachine elasticmachine added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label Feb 12, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

@DaveCTurner
Copy link
Contributor Author

Unfortunately I don't have a good way to test this against a real Azure account right now, and ./gradlew :plugins:repository-azure:check doesn't seem to do anything useful for me so I'm just opening this as a draft for now.

@DaveCTurner DaveCTurner marked this pull request as draft February 12, 2021 20:51
@DaveCTurner
Copy link
Contributor Author

./gradlew :plugins:repository-azure:check doesn't seem to do anything useful for me

I worked this out, it's because I was running it on a machine that doesn't have Docker.

@DaveCTurner
Copy link
Contributor Author

@elasticmachine please run elasticsearch-ci/2

@DaveCTurner DaveCTurner marked this pull request as ready for review February 15, 2021 10:50
Copy link
Contributor

@fcofdez fcofdez left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for fixing this @DaveCTurner! I've run the test suites a few times agains azure (using a really small multi-block size threshold) and it passed.

DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Feb 15, 2021
Today we represent block IDs sent to Azure using the URL-safe base-64
encoding. This makes sense: these IDs appear in URLs. It turns out that
Azure rejects this encoding for block IDs and instead demands that they
are represented using the regular, URL-unsafe, base-64 encoding instead,
then further wrapped in %-encoding to deal with the URL-unsafe
characters that inevitably result.

Relates elastic#66489
Backport of elastic#68957
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Feb 15, 2021
Today we represent block IDs sent to Azure using the URL-safe base-64
encoding. This makes sense: these IDs appear in URLs. It turns out that
Azure rejects this encoding for block IDs and instead demands that they
are represented using the regular, URL-unsafe, base-64 encoding instead,
then further wrapped in %-encoding to deal with the URL-unsafe
characters that inevitably result.

Relates elastic#66489
Backport of elastic#68957
@DaveCTurner DaveCTurner merged commit b3d5d32 into elastic:master Feb 15, 2021
@DaveCTurner DaveCTurner deleted the 2021-02-12-azure-base64 branch February 15, 2021 11:34
@DaveCTurner
Copy link
Contributor Author

Merged despite unrelated CI failures, with approval from @jasontedor.

DaveCTurner added a commit that referenced this pull request Feb 15, 2021
Today we represent block IDs sent to Azure using the URL-safe base-64
encoding. This makes sense: these IDs appear in URLs. It turns out that
Azure rejects this encoding for block IDs and instead demands that they
are represented using the regular, URL-unsafe, base-64 encoding instead,
then further wrapped in %-encoding to deal with the URL-unsafe
characters that inevitably result.

Relates #66489
Backport of #68957
DaveCTurner added a commit that referenced this pull request Feb 15, 2021
Today we represent block IDs sent to Azure using the URL-safe base-64
encoding. This makes sense: these IDs appear in URLs. It turns out that
Azure rejects this encoding for block IDs and instead demands that they
are represented using the regular, URL-unsafe, base-64 encoding instead,
then further wrapped in %-encoding to deal with the URL-unsafe
characters that inevitably result.

Relates #66489
Backport of #68957
@idegtiarenko
Copy link
Contributor

idegtiarenko commented Feb 17, 2021

Hello @DaveCTurner , I believe according to the merge commit this is not yet in any releases, is it?
Is it scheduled to be part of 7.11.2?

UPD, just found this comment: https://github.com/elastic/dev/issues/1617#issuecomment-779160469

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. v7.11.1 v7.12.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants