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

[fix][doc]: add the primary auth config on cluster failover example #620

Merged
merged 4 commits into from
Jul 24, 2023

Conversation

ericsyh
Copy link
Contributor

@ericsyh ericsyh commented Jun 27, 2023

Motivation

It's quite weird that on the https://pulsar.apache.org/docs/3.0.x/client-libraries-cluster-level-failover/#automatic-failover code example only includes the secondary cluster auth config without the primary cluster auth.

And as my check on test code https://github.com/apache/pulsar/blob/2b01f83ea4f3ab7b22f4ea2b0290cc183e79bbb8/pulsar-client/src/test/java/org/apache/pulsar/client/impl/AutoClusterFailoverTest.java#L73 it has the primaryAuthentication config creation.

This PR adds doc for #xyz

This PR fixes #xyz

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

@github-actions github-actions bot added the doc Improvements or additions to documentation label Jun 27, 2023
@Anonymitaet
Copy link
Member

@hangc0276
Could you please review this PR from a technical perspective? Thank you!

@Anonymitaet
Copy link
Member

Ping @hangc0276 to review, thanks!

@Anonymitaet Anonymitaet added this to the 3.1.0 milestone Jun 30, 2023
@Anonymitaet
Copy link
Member

Ping @hangc0276 to review, thanks!

String secondaryTlsTrustCertsFilePath = "secondary/path";
Authentication primaryAuthentication = AuthenticationFactory.create(

Choose a reason for hiding this comment

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

The primaryAuthentication is in pulsarClient initiation and we need to add a configuration for pulsar client creation. Otherwise, users will be confused by primaryAuthentication and primaryTlsTrustCertsFilePath initiated without use.

PulsarClient pulsarClient = PulsarClient.builder()
            .authentication(primaryAuthentication)
            .tlsTrustCertsFilePath(primaryTlsTrustCertsFilePath)
            .serviceUrl("pulsar://localhost:6650")
            .build();

@@ -76,6 +83,8 @@ Parameter|Default value|Required?|Description
`failoverDelay`|N/A|Yes|The delay before the Pulsar client switches from the primary cluster to the backup cluster.<br /><br />Automatic failover is controlled by a probe task: <br />1) The probe task first checks the health status of the primary cluster. <br /> 2) If the probe task finds the continuous failure time of the primary cluster exceeds `failoverDelayMs`, it switches the Pulsar client to the backup cluster.
`switchBackDelay`|N/A|Yes|The delay before the Pulsar client switches from the backup cluster to the primary cluster.<br /><br />Automatic failover switchover is controlled by a probe task: <br /> 1) After the Pulsar client switches from the primary cluster to the backup cluster, the probe task continues to check the status of the primary cluster. <br /> 2) If the primary cluster functions well and continuously remains active longer than `switchBackDelay`, the Pulsar client switches back to the primary cluster.
`checkInterval`|30s|No|Frequency of performing a probe task (in seconds).
`primaryTlsTrustCertsFilePath` |N/A|No|Path to the trusted TLS certificate file of the primary cluster.

Choose a reason for hiding this comment

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

This is not the AutoClusterFailover's configuration, they are PulsarClient's configuration. I think we do not need to add here

Copy link
Contributor Author

@ericsyh ericsyh Jul 16, 2023

Choose a reason for hiding this comment

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

@Anonymitaet Anonymitaet merged commit 7055001 into apache:main Jul 24, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants