Skip to content

Add properties required by Datastax Cassandra java driver v4#12212

Merged
ebyhr merged 1 commit intotrinodb:masterfrom
findinpath:cassandra-v4-docs
May 3, 2022
Merged

Add properties required by Datastax Cassandra java driver v4#12212
ebyhr merged 1 commit intotrinodb:masterfrom
findinpath:cassandra-v4-docs

Conversation

@findinpath
Copy link
Copy Markdown
Contributor

Description

After transitioning to Cassandra java driver v4, the following properties
are required:

  • cassandra.load-policy.use-dc-aware
  • cassandra.load-policy.dc-aware.local-dc

Is this change a fix, improvement, new feature, refactoring, or other?

Documentation fix

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)

Cassandra connector

How would you describe this change to a non-technical end user or system administrator?

This PR adds a bit more documentation for the users of trino-cassandra after introducing #7828

Related issues, pull requests, and links

Related PR: #7828

Documentation

( ) No documentation is needed.
(x) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

(x) No release notes entries required.
( ) Release notes entries required with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label May 2, 2022
@findinpath findinpath added the docs label May 2, 2022
@findinpath findinpath requested review from ebyhr and mosabua May 2, 2022 20:39
Copy link
Copy Markdown
Member

@mosabua mosabua May 2, 2022

Choose a reason for hiding this comment

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

So thats weird... its required but it has a default value? So can I leave it out and then it will just use the default? If that is the case.. its NOT required.. .. we need to clarify this here and in the release notes text

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 has default value, but we need to enable it explicitly. We will consider changing the default value later.

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.

so this is NOT required then... lets remove it form the code snippet earlier

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.

As we talked offline, changing the default value (false → true) is also fine to me.

Copy link
Copy Markdown
Contributor Author

@findinpath findinpath May 3, 2022

Choose a reason for hiding this comment

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

At the time of this writing, we use

driverConfigLoaderBuilder.withString(DefaultDriverOption.LOAD_BALANCING_POLICY_CLASS, DefaultLoadBalancingPolicy.class.getName());

as the sole load balancing policy class:

should almost always be used; it requires a local datacenter to be specified either programmatically when creating the session, or via the configuration option: datastax-java-driver.basic.load-balancing-policy.local-datacenter. It can also use a highly efficient slow replica avoidance mechanism, which is by default enabled – see the option: datastax-java-driver.basic.load-balancing-policy.slow-replica-avoidance.

See https://github.com/datastax/java-driver/blob/4.x/core/src/main/resources/reference.conf

In the current context, I don't know whether the property cassandra.load-policy.use-dc-aware still makes sense.

Copy link
Copy Markdown
Member

@mosabua mosabua left a comment

Choose a reason for hiding this comment

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

Needs clarification of what is actually required in docs and potentially in release notes

@findinpath findinpath force-pushed the cassandra-v4-docs branch from e778bdd to d7a7304 Compare May 3, 2022 04:25
@findinpath findinpath requested review from ebyhr and mosabua May 3, 2022 04:26
@findinpath findinpath force-pushed the cassandra-v4-docs branch 2 times, most recently from d607635 to 67e90c0 Compare May 3, 2022 09:06
After transitioning to Cassandra java driver v4, the property
`cassandra.load-policy.dc-aware.local-dc` is required for the
`DefaultLoadBalancingPolicy`.
@findinpath findinpath force-pushed the cassandra-v4-docs branch from 67e90c0 to c19a06b Compare May 3, 2022 11:36
datacenter, defaults to ``true``.

``cassandra.load-policy.dc-aware.local-dc`` The name of the local datacenter for ``DCAwareRoundRobinPolicy``.
``cassandra.load-policy.dc-aware.local-dc`` The name of the datacenter considered "local".
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.

Ideally we say this is required when use-dc-aware is true... but I am fine with the current status as well

@ebyhr ebyhr merged commit 9e9c6f0 into trinodb:master May 3, 2022
@github-actions github-actions bot added this to the 380 milestone May 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants