Skip to content

Add session refresh interval in Cassandra connector#8884

Closed
ebyhr wants to merge 3 commits intotrinodb:masterfrom
ebyhr:ebi/cassandra-session-ttl
Closed

Add session refresh interval in Cassandra connector#8884
ebyhr wants to merge 3 commits intotrinodb:masterfrom
ebyhr:ebi/cassandra-session-ttl

Conversation

@ebyhr
Copy link
Copy Markdown
Member

@ebyhr ebyhr commented Aug 16, 2021

Fixes #4838

I considered extending BaseConnectorTest and BaseConnectorSmokeTest for Yugabyte tests at first, but there were some failures due to lack of comment table properties in Yugabyte. Cassandra connector uses comment to mange hidden columns.

Also, excluded the logic to populate TPCH data in Yugabyte query runner because it was flaky, returning the wrong row number just after insert.

@cla-bot cla-bot bot added the cla-signed label Aug 16, 2021
@ebyhr ebyhr force-pushed the ebi/cassandra-session-ttl branch from 32f4b59 to e3543d1 Compare August 16, 2021 01:26
Copy link
Copy Markdown
Member

@hashhar hashhar left a comment

Choose a reason for hiding this comment

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

Looks good. Is this required for #5708?

@ebyhr ebyhr force-pushed the ebi/cassandra-session-ttl branch from e3543d1 to 2431004 Compare December 6, 2021 10:46
Copy link
Copy Markdown
Member

@hashhar hashhar left a comment

Choose a reason for hiding this comment

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

Can you mention in 2nd commit's message that it's to make it work with Yugabyte CQL tables?

Also, some recommendation in docs to enable set a session refresh interval when connecting to Yugabyte would be helpful.

LGTM otherwise.

@ebyhr
Copy link
Copy Markdown
Member Author

ebyhr commented Jan 18, 2022

@findinpath Can you test if we need this config properties for Yugabyte CQL even after #7828?

@findinpath
Copy link
Copy Markdown
Contributor

@ebyhr I rebased your changes on top of the cassandra v4 changes and created a new branch
https://github.com/findinpath/trino/tree/cassandra-v4-yugabyte

It doesn't quite work out of the box.

com.datastax.oss.driver.api.core.session.Session#getMetadata states

Do not call it once and store the result, because it is a frozen snapshot that will become stale over time.

After doing the change:
session.refreshSchema().getKeyspaces() in io.trino.plugin.cassandra.CassandraSession#getCaseSensitiveSchemaNames
there is another exception thrown:

Caused by: java.lang.IllegalStateException: Can't find referenced user type jsonb
	at com.datastax.oss.driver.internal.core.metadata.schema.parsing.DataTypeCqlNameParser.parse(DataTypeCqlNameParser.java:84)
	at com.datastax.oss.driver.internal.core.metadata.schema.parsing.DataTypeCqlNameParser.parse(DataTypeCqlNameParser.java:51)
	at com.datastax.oss.driver.internal.core.metadata.schema.parsing.TableParser.parseTable(TableParser.java:170)
	at com.datastax.oss.driver.internal.core.metadata.schema.parsing.CassandraSchemaParser.parseTables(CassandraSchemaParser.java:178)
	at com.datastax.oss.driver.internal.core.metadata.schema.parsing.CassandraSchemaParser.parseKeyspace(CassandraSchemaParser.java:129)
	at com.datastax.oss.driver.internal.core.metadata.schema.parsing.CassandraSchemaParser.parse(CassandraSchemaParser.java:75)

@ebyhr
Copy link
Copy Markdown
Member Author

ebyhr commented Jan 19, 2022

@findinpath Thanks!

Let me close this PR because it would be better to add the dedicated connector for Yugabyte. This change might be still required when adding the connector, but I wouldn't merge at this time.

@ebyhr ebyhr closed this Jan 19, 2022
@ebyhr ebyhr deleted the ebi/cassandra-session-ttl branch January 19, 2022 01:19
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.

Cassandra connector doesn't discover new keyspaces/tables with Yugabyte

3 participants