Skip to content
This repository has been archived by the owner on Sep 30, 2024. It is now read-only.

ConsulTxnStore: batch KV updates by key-prefix to avoid ops limit #1311

Merged
merged 16 commits into from
Apr 4, 2021

Conversation

timvaillancourt
Copy link
Contributor

@timvaillancourt timvaillancourt commented Feb 17, 2021

Resolves issue: #1302

Description

This PR batches KVPair updates by key-prefix in to resolve errors for updates with more than 64 KVPairs. KVPairs are batched by key-prefix to ensure keys for a single "cluster" are updated in a single transaction atomically as only 5 x KVPair updates are necessary (< 64)

Details:

  • Add ConsulMaxKVsPerTransaction config-param for increasing transaction size, default to 5
    • Default 5 causes 1 x transaction per cluster as there are 5 x KVs each
    • Add logic to ensure the value is between 5 (# of KVs/cluster) and 64 (a Consul API limit)
  • Group KVPairs by key-prefix in .DistributePairs()
    • Example mysql/master/cluster1, mysql/master/cluster1/hostname, mysql/master/cluster1/ipv4, mysql/master/cluster1/ipv6 and mysql/master/cluster1/port is grouped into a single transaction at the default ConsulMaxKVsPerTransaction=5
  • Concurrently perform grouped KVPair updates with goroutines + sync.WaitGroup
  • Testing
    • Add > 64-op transaction error Transaction contains too many operations... to mock Consul server in unit test
    • Add test for .DistributePairs()
  • docs/kv.md update

I'm not 100% sure if my changes to go/config/config.go fit with the theme. I also don't like that I made a constant for the number of KVPairs per "cluster". Any suggestions appreciated!

Example debug-logging from .DistributePairs():

2021-02-17 10:25:35 DEBUG consulTxnStore.DistributePairs(): datacenter: dc1; getTxns: 0, setTxns: 0, skipped: 530, existing: 0, written: 0, failed: 0
2021-02-17 10:25:35 DEBUG consulTxnStore.DistributePairs(): datacenter: dc2; getTxns: 0, setTxns: 0, skipped: 530, existing: 0, written: 0, failed: 0
2021-02-17 10:25:35 DEBUG consulTxnStore.DistributePairs(): datacenter: dc3; getTxns: 0, setTxns: 0, skipped: 530, existing: 0, written: 0, failed: 0
2021-02-17 10:25:35 DEBUG consulTxnStore.DistributePairs(): datacenter: dc4; getTxns: 0, setTxns: 0, skipped: 530, existing: 0, written: 0, failed: 0

cc @shlomi-noach

@timvaillancourt
Copy link
Contributor Author

@shlomi-noach I've confirmed this fixes the Consul Transaction support for environments that exceed the 64 max txn ops

I think this is safe to merge 👍

Copy link
Collaborator

@shlomi-noach shlomi-noach left a comment

Choose a reason for hiding this comment

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

thank you

@shlomi-noach shlomi-noach merged commit 4722e3f into openark:master Apr 4, 2021
@timvaillancourt timvaillancourt deleted the consul-txn-batch branch April 28, 2021 23:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants