Skip to content

Document -max_replication_lag_allowed flag#957

Merged
mattlord merged 5 commits intoprodfrom
rn-max-allowed-lag-9538
Feb 16, 2022
Merged

Document -max_replication_lag_allowed flag#957
mattlord merged 5 commits intoprodfrom
rn-max-allowed-lag-9538

Conversation

@rohit-nayak-ps
Copy link
Copy Markdown
Member

@rohit-nayak-ps rohit-nayak-ps commented Jan 30, 2022

@netlify
Copy link
Copy Markdown

netlify bot commented Jan 30, 2022

✔️ Deploy Preview for vitess ready!

🔨 Explore the source changes: f7edebd

🔍 Inspect the deploy log: https://app.netlify.com/sites/vitess/deploys/620d4fdcb3e6c200070822ac

😎 Browse the preview: https://deploy-preview-957--vitess.netlify.app

@mattlord mattlord self-requested a review February 1, 2022 01:55

<div class="cmd">

While switching traffic ensure that the VReplication lag for the workflow is less than this duration, otherwise
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

it's number of seconds, right? IMO, saying duration here may be confusing as many flags take a golang duration value like "5s" but here it would just be 5.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It is a duration maxReplicationLagAllowed := subFlags.Duration("max_replication_lag_allowed"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah, OK. So instead it's this that I think is confusing/misleading: https://github.com/vitessio/vitess/blob/6443bb502885dda91af6b5337e7bc682cb2861d9/go/vt/vtctl/vtctl.go#L2299

the "in seconds" part. No?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, the comment for the flag is incorrect, will fix in my next PR.

@mattlord mattlord self-requested a review February 2, 2022 06:34
Copy link
Copy Markdown
Member

@mattlord mattlord left a comment

Choose a reason for hiding this comment

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

In addition to updating the flag name to max_replication_lag_allowed, could you please copy these changes to the new 14.0 versions of the pages?

  • content/en/docs/14.0/reference/vreplication/flags.md
  • content/en/docs/14.0/reference/vreplication/switchtraffic.md

Those pages started as carbon copies of the 13.0 ones, so we can just copy them as-is to the 14.0 directory.

Thank you!

@mattlord mattlord changed the title Document -max_transaction_lag_allowed flag Document -max_replication_lag_allowed flag Feb 2, 2022
Signed-off-by: Rohit Nayak <rohit@planetscale.com>
Signed-off-by: Rohit Nayak <rohit@planetscale.com>
Signed-off-by: Rohit Nayak <rohit@planetscale.com>
Signed-off-by: Rohit Nayak <rohit@planetscale.com>
Copy link
Copy Markdown
Member

@mattlord mattlord left a comment

Choose a reason for hiding this comment

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

Thanks!

@mattlord mattlord force-pushed the rn-max-allowed-lag-9538 branch 2 times, most recently from c52456a to 0957c24 Compare February 16, 2022 19:22
Signed-off-by: Matt Lord <mattalord@gmail.com>
@mattlord mattlord force-pushed the rn-max-allowed-lag-9538 branch from 0957c24 to f7edebd Compare February 16, 2022 19:26
@mattlord mattlord merged commit 3a32361 into prod Feb 16, 2022
@mattlord mattlord deleted the rn-max-allowed-lag-9538 branch February 16, 2022 19:32
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.

2 participants