Skip to content

Core: add az awareness to read strategy#2539

Merged
adarovadya merged 10 commits intovalkey-io:release-1.2from
adarovadya:az-awerness-1.2
Nov 18, 2024
Merged

Core: add az awareness to read strategy#2539
adarovadya merged 10 commits intovalkey-io:release-1.2from
adarovadya:az-awerness-1.2

Conversation

@adarovadya
Copy link
Copy Markdown
Collaborator

@adarovadya adarovadya commented Oct 29, 2024

Issue link

This Pull Request is linked to issue (URL): #643

This PR adding support to AZ Affinity strategy for CME and CMD.
Only version Valkey-8.0 and above support AZ affinity strategy, since only from this version the info command supports the availability-zone property, which is required to support this feature.

In addition, as for now we added support only to the Async Client. Support for the Sync Client has been added to our backlog.

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one issue.
  • Commit message has a detailed description of what changed and why.
  • Tests are added or updated.
  • CHANGELOG.md and documentation files are updated.
  • Destination branch is correct - main or release
  • Commits will be squashed upon merging.

@adarovadya adarovadya changed the title redis-core: add az awareness to read strategy redis-core: add az awareness to read strategy- DRAFT Oct 29, 2024
@asafpamzn asafpamzn mentioned this pull request Oct 30, 2024
@adarovadya adarovadya force-pushed the az-awerness-1.2 branch 3 times, most recently from ec034fe to 9d5e64d Compare November 7, 2024 16:00
@adarovadya adarovadya changed the title redis-core: add az awareness to read strategy- DRAFT redis-core: add az awareness to read strategy Nov 7, 2024
@adarovadya adarovadya changed the title redis-core: add az awareness to read strategy Core: add az awareness to read strategy Nov 7, 2024
@adarovadya adarovadya force-pushed the az-awerness-1.2 branch 5 times, most recently from 518c991 to 5edc891 Compare November 7, 2024 17:32
@adarovadya adarovadya marked this pull request as ready for review November 7, 2024 17:38
@adarovadya adarovadya requested a review from a team as a code owner November 7, 2024 17:38
@adarovadya adarovadya force-pushed the az-awerness-1.2 branch 11 times, most recently from 0b3de2c to 47691cf Compare November 12, 2024 08:57
@adarovadya adarovadya force-pushed the az-awerness-1.2 branch 2 times, most recently from 8f419bc to a72f324 Compare November 14, 2024 11:29
@eifrah-aws eifrah-aws self-requested a review November 14, 2024 11:59
Ok(value) => {
let info_dict: InfoDict = FromRedisValue::from_redis_value(&value)?;
if let Some(node_az) = info_dict.get::<String>("availability_zone") {
con.set_az(Some(node_az));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would also add a debug message stating that the property "availability_zone" could not be found for this redis server

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@eifrah-aws I dont think its required - not having AZ is ok (redis/older versions)

}

fn get_connection(&self, readonly: bool) -> &ReconnectingConnection {
async fn round_robin_read_from_replica_az_awareness(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am worried that if we fix a bug in this method, we will also need to fix the one that is a copy - paste of this one above

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

what is the bug?

Adar Ovadia added 6 commits November 16, 2024 20:05
Signed-off-by: Adar Ovadia <adarov@amazon.com>
Signed-off-by: Adar Ovadia <adarov@amazon.com>
Signed-off-by: Adar Ovadia <adarov@amazon.com>
Signed-off-by: Adar Ovadia <adarov@amazon.com>
…han 7.1 version

Signed-off-by: Adar Ovadia <adarov@amazon.com>
Signed-off-by: Adar Ovadia <adarov@amazon.com>
Signed-off-by: Adar Ovadia <adarov@amazon.com>
Adar Ovadia added 2 commits November 17, 2024 11:59
Signed-off-by: Adar Ovadia <adarov@amazon.com>
Signed-off-by: Adar Ovadia <adarov@amazon.com>
Signed-off-by: Adar Ovadia <adarov@amazon.com>
@adarovadya adarovadya merged commit ddd98cc into valkey-io:release-1.2 Nov 18, 2024
@adarovadya adarovadya deleted the az-awerness-1.2 branch January 15, 2025 11:50
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.

5 participants