[cassandra] Remove SASI index from dependencies table #793

vprithvi · 2018-04-27T19:58:11Z

remove SASI indexes so that users may use older Cassandra versions or ScyllaDB
provide a migration script for current users

ref #790
Signed-off-by: Prithvi Raj p.r@uber.com

yurishkuro · 2018-04-27T20:10:13Z

plugin/storage/cassandra/schema/v001.cql.tmpl

-    WITH compaction = {
+    PRIMARY KEY (bucket, ts)
+) WITH CLUSTERING ORDER BY (ts DESC)
+    AND compaction = {
        'min_threshold': '4',
        'max_threshold': '32',
        'class': 'org.apache.cassandra.db.compaction.SizeTieredCompactionStrategy'


actually delete SASI below

yurishkuro · 2018-04-27T20:10:59Z

plugin/storage/cassandra/schema/v001.cql.tmpl

-    ts          timestamp,
-    ts_index    timestamp,
+    ts           timestamp,
+    date_bucket  text


shouldn't this be int?

I initially had buckets in a different format, this can be int, or bigint if we anticipate smaller buckets in the future.

yurishkuro · 2018-04-27T20:11:34Z

plugin/storage/cassandra/schema/v001.cql.tmpl

-    ts          timestamp,
-    ts_index    timestamp,
+    ts           timestamp,
+    date_bucket  text


it's customary to list PK fields at the top

yurishkuro · 2018-04-27T20:12:27Z

plugin/storage/cassandra/schema/v001.cql.tmpl

-    PRIMARY KEY (ts)
-)
-    WITH compaction = {
+    PRIMARY KEY (bucket, ts)


s/bucket/data_bucket

coveralls · 2018-05-02T21:33:23Z

Coverage remained the same at 100.0% when pulling 7d98f69 on remove-SASI-index into 4106c29 on master.

- Use traditional indexes and a date bucket field instead - This allows people to use older versions of cassandra Signed-off-by: Prithvi Raj <p.r@uber.com>

isaachier

Looks good. You might want to run shellcheck on the bash script.

isaachier · 2018-05-07T15:11:59Z

plugin/storage/cassandra/schema/migration/v001tov002.sh

+#!/usr/bin/env bash
+
+function usage {
+    >&2 echo "Error: $1"


Any reason not to use a heredoc?

Good question, let me try it out

isaachier · 2018-05-07T15:18:59Z

plugin/storage/cassandra/dependencystore/storage.go

@@ -93,6 +98,14 @@ func (s *DependencyStore) GetDependencies(endTs time.Time, lookback time.Duratio
 	return mDependency, nil
 }

+func getDepSelectString(startTs time.Time, endTs time.Time) string {


Cassandra has no date range support?

Cassandra does not support range queries on partition keys

OK that explains it.

isaachier · 2018-05-07T15:22:10Z

plugin/storage/cassandra/schema/migration/v001tov002.sh

+confirm() {
+    read -r -p "${1:-Are you sure? [y/N]} " response
+    case "$response" in
+        [yY][eE][sS]|[yY])


You can probably just do if [[ "$(echo $response | tr 'a-z' 'A-Z')" =~ "Y(ES)?" ]].

I don't think that would work

I'm not saying it's necessarily easier, but why wouldn't it work?

I don't know, I didn't spend time debugging it. Does it work for you?

bash-3.2$ response=Y && if [[ "$(echo $response | tr 'a-z' 'A-Z')" =~ "Y(ES)?" ]]; then echo "passed"; fi bash-3.2$

Right. I think bash regex might just be awful.

black-adder · 2018-05-07T15:46:36Z

Changes look fine, what's our strategy for rolling this out? We going to wait until a major release? If not, we probably want this to be backwards compatible.

vprithvi · 2018-05-07T15:54:30Z

Changes look fine, what's our strategy for rolling this out? We going to wait until a major release? If not, we probably want this to be backwards compatible.

The strategy for rolling out is for people to stop the spark dependency job, which stops writes to the dependency table, run the migration script, and start a new version of the spark job. (I'm writing this up in a readme)

vprithvi · 2018-05-22T22:03:28Z

I had an offline discussion with @black-adder and decided that it makes sense to add a new Keyspace to accomplish some backward compatibility.

The new dependencies table will live in the new keyspace, so that jaeger-query instances that are not updated can still access old dependencies at the older location.

yurishkuro · 2018-05-22T22:09:44Z

I think I'd need more info. The main concern is data compatibility, not running outdated query - upgrading components is the easy part.

vprithvi · 2018-05-22T22:18:45Z

I think I'd need more info.

The idea is the we will create a new Keyspace as follows:
jaeger_analytics_$dc_$version

This keyspace contains the new dependency table.

The migration path looks like the following:

Run a script to create the new keyspace and schema
Run the updated spark-dependencies job (which writes to the new schema)
Run a data migration script to transfer old dependencies from the old table to the new table
Update jaeger-query
Kill the old spark-dependencies job
Delete the old dependencies table in the old keyspace

The main benefit offered here is that dependencies will be available throughout the update/migration process because the old spark-dependencies job still writes in the old data format.

yurishkuro · 2018-05-23T14:57:12Z

plugin/storage/cassandra/schema/migration/v001tov002.sh

+
+cqlsh -e "CREATE TABLE $keyspace.dependencies (
+    ts           timestamp,
+    date_bucket  bigint,


partition key first

vprithvi · 2019-02-13T21:53:29Z

Superseded by #1328

vprithvi requested review from black-adder, pavolloffay and yurishkuro as code owners April 27, 2018 19:58

ghost assigned vprithvi Apr 27, 2018

ghost added the review label Apr 27, 2018

yurishkuro reviewed Apr 27, 2018

View reviewed changes

vprithvi force-pushed the remove-SASI-index branch 2 times, most recently from f07db20 to 5e58764 Compare May 2, 2018 20:56

vprithvi force-pushed the remove-SASI-index branch from 5e58764 to ae7e823 Compare May 4, 2018 22:01

Remove SASI indexes

a19184f

- Use traditional indexes and a date bucket field instead - This allows people to use older versions of cassandra Signed-off-by: Prithvi Raj <p.r@uber.com>

vprithvi force-pushed the remove-SASI-index branch from ae7e823 to a19184f Compare May 4, 2018 22:32

vprithvi changed the title ~~WIP: Remove SASI indexes~~ Remove SASI indexes May 7, 2018

Merge branch 'master' into remove-SASI-index

7d98f69

isaachier approved these changes May 7, 2018

View reviewed changes

black-adder mentioned this pull request May 8, 2018

[META] How to support breaking changes prior to 2.0.0 #811

Closed

yurishkuro reviewed May 23, 2018

View reviewed changes

yurishkuro mentioned this pull request Oct 15, 2018

How to manage Cassandra schemas #1119

Open

yurishkuro changed the title ~~Remove SASI indexes~~ [cassandra] Remove SASI index from dependencies table Nov 13, 2018

yurishkuro mentioned this pull request Nov 13, 2018

Why not use ScyllaDB? #197

Closed

black-adder mentioned this pull request Feb 11, 2019

Remove SASI indices #1328

Merged

2 tasks

vprithvi closed this Feb 13, 2019

ghost removed the review label Feb 13, 2019

pavolloffay deleted the remove-SASI-index branch August 27, 2019 08:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[cassandra] Remove SASI index from dependencies table #793

[cassandra] Remove SASI index from dependencies table #793

vprithvi commented Apr 27, 2018 •

edited

Loading

yurishkuro Apr 27, 2018

yurishkuro Apr 27, 2018

vprithvi Apr 27, 2018

yurishkuro Apr 27, 2018

yurishkuro Apr 27, 2018

coveralls commented May 2, 2018 •

edited

Loading

isaachier left a comment

isaachier May 7, 2018

vprithvi May 7, 2018

isaachier May 7, 2018

vprithvi May 7, 2018

isaachier May 7, 2018

isaachier May 7, 2018 •

edited

Loading

vprithvi May 7, 2018

isaachier May 7, 2018

vprithvi May 7, 2018 •

edited

Loading

isaachier May 7, 2018

black-adder commented May 7, 2018

vprithvi commented May 7, 2018

vprithvi commented May 22, 2018

yurishkuro commented May 22, 2018

vprithvi commented May 22, 2018

yurishkuro May 23, 2018

vprithvi commented Feb 13, 2019

[cassandra] Remove SASI index from dependencies table #793

[cassandra] Remove SASI index from dependencies table #793

Conversation

vprithvi commented Apr 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented May 2, 2018 • edited Loading

isaachier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

isaachier May 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vprithvi May 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

black-adder commented May 7, 2018

vprithvi commented May 7, 2018

vprithvi commented May 22, 2018

yurishkuro commented May 22, 2018

vprithvi commented May 22, 2018

Choose a reason for hiding this comment

vprithvi commented Feb 13, 2019

vprithvi commented Apr 27, 2018 •

edited

Loading

coveralls commented May 2, 2018 •

edited

Loading

isaachier May 7, 2018 •

edited

Loading

vprithvi May 7, 2018 •

edited

Loading