Deprecate Thrift API #3811

duarten · 2018-10-03T20:01:23Z

We should deprecate the Thrift API with the goal of removing the code in some future version. The reasons for this are:

Past Scylla version have Thrift support, and work correctly;
We are unlikely to invest effort in making new features like SI and LWT work from Thrift;
Projects like KairosDB that supported Scylla as a backend and used Thrift have since moved to CQL;
Less code means faster compile times!

glommer · 2018-10-03T20:09:39Z

Disagree. There are some folks that are still using thrift, believe it or not. Some of them don't want to rewrite applications and would consider moving to Scylla instead of newer Cassandra.

We shouldn't deprecate things without very good reason. APIs are particularly problematic.

The "past scylla versions have thrift" is not a good argument in my view. Those past versions exist, but they are unsupported, so what good is that ?

We obviously don't have to invest more time in it, and should only fix critical bugs. But I see deprecation as the wrong thing to do.

nyh · 2018-10-04T08:10:45Z

@glommer please note that Cassandra dropped Thrift support in 4.0 (see https://issues.apache.org/jira/browse/CASSANDRA-11115) in a move that has been planned for two years with out too much opposition. So users who insist on continuing to using thrift would need to do more than "consider" to use Scylla instead of newer Cassandra - because newer Cassandra will not support it at all.
Personally, I think we should keep Thrift for a while longer, but mark it deprecated and definitely not recommended for new projects, and we don't need to support anything more in it than we already do.

arganzheng · 2019-06-14T13:28:49Z

Without Thrift API, how can we insert record without define every column before we can insert it.

nyh · 2021-03-29T10:12:53Z

After #8336, the Thrift protocol is no longer enabled by default. But it's still not officially deprecated.

mykaul · 2022-12-11T13:45:34Z

It's almost 2023, perhaps it's the year of deprecation of Thrift!

DoronArazii · 2023-01-24T10:40:51Z

@mykaul is it planned for 5.2?
Which team has the commitment?

mykaul · 2023-01-24T10:42:01Z

@mykaul is it planned for 5.2? Which team has the commitment?

No critical, I assume @eliransin , could be postponed to 5.3.

nyh · 2023-02-15T13:22:16Z

Issues #1478 and #9911, which asked to improve testing of Thrift but never done - were closed recently.
Not having good tests is yet another reason to drop Thrift support.

eliransin · 2023-02-19T09:12:11Z

@mykaul , I am not sure about the decision. Should we deprecate thrift? If so, maybe in order to deprecate thrift in 5.3 we should at least start by issuing a warning that it is going to be deprecated in 5.2 and the actually deprecate it in 5.3?

mykaul · 2023-02-19T09:28:43Z

@mykaul , I am not sure about the decision. Should we deprecate thrift? If so, maybe in order to deprecate thrift in 5.3 we should at least start by issuing a warning that it is going to be deprecated in 5.2 and the actually deprecate it in 5.3?

We can issue a warning, including in the release notes, in 5.2, and do it in 5.3.

nyh · 2023-02-19T09:28:48Z

I think that dropping Thrift is not really a technical decision - technically Thrift is bad and not needed and not tested enough (the issues to fix that we. I think it's a product/business decision:

Do we know of users who use Thrift or want to use it? How many such users?
Is our unique Thrift support (which Cassandra no longer supports since 4.0) attracting users to Scylla?

@eliransin for two years now (since ccc75bf) Thrift is disabled in Scylla by default and needs to be enabled manually with the start_rpc configuration option. This means we can indeed warn users when Scylla starts with that option enabled - warn that this option will be going away soon (?).

The documentation that I wrote (docs/dev/protocols.md) already un-recommends using Thrift for three years already:

The Apache Thrift protocol was early Cassandra's client protocol, until it was superceded in Cassandra 1.2 with the binary CQL protocol. Thrift was still nominally supported by both Cassandra and Scylla for many years, but was recently dropped in Cassandra (version 4.0) and is likely to be dropped by Scylla in the future as well, so it is not recommended for new applications.

Maybe we needed similar statements in other places in the documentation as well. CC @tzach @annastuchlik

This commit adds the information that Thrift is deprecated (both in ScyllaDB and Cassandra) to the Cassandra compatibility page. Refs: scylladb#3811

thrift support was deprecated since ScyllaDB 5.2 > Thrift API - legacy ScyllaDB (and Apache Cassandra) API is > deprecated and will be removed in followup release. Thrift has > been disabled by default. so let's drop it. in this change, * thrift protocol support is dropped * all references to thrift support in document are dropped * the "thrift_version" column in system.local table is preserved for backward compatibility, as we could load from an existing system.local table which still contains this clolumn, so we need to write this column as well. * "/storage_service/rpc_server" is only preserved for backward compatibility with java-based nodetool. Fixes scylladb#3811 Fixes scylladb#18416 Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>

cassandra-cli was removed in Cassandra 2.2.0. and it was replaced by cqlsh in newer releases. actually, scylla-tools does not build or package this command line tool at all. so let's drop the related helpers for accessing this tool. in a following-up change, we will drop the thrift related APIs. Refs scylladb/scylladb#3811 Refs scylladb/scylladb#18416 Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>

thrift was deprecated in both ScyllaDB and Cassandra. so there is no need to pass it around. to be compatible with existing users of the ccmlib, the thrift related parameters are preserved in public intefaces, but they are not passed down anymore. in some places, we enforce that the host on which thrift protocol is served should be identical to that of binary. and actually, scylla always use the same host address for both thrift and binary protocols. so we replace the address like `self.network_interfaces['thrift'][0]` with `self.network_interfaces['binary'][0]`. Refs scylladb/scylladb#3811 Refs scylladb/scylladb#18416 Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>

thrift support was deprecated since ScyllaDB 5.2 > Thrift API - legacy ScyllaDB (and Apache Cassandra) API is > deprecated and will be removed in followup release. Thrift has > been disabled by default. so let's drop it. in this change, * thrift protocol support is dropped * all references to thrift support in document are dropped * the "thrift_version" column in system.local table is preserved for backward compatibility, as we could load from an existing system.local table which still contains this clolumn, so we need to write this column as well. * "/storage_service/rpc_server" is only preserved for backward compatibility with java-based nodetool. Fixes scylladb#3811 Fixes scylladb#18416 Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>

thrift was deprecated in both ScyllaDB and Cassandra. so there is no need to pass it around. to be compatible with existing users of the ccmlib, the thrift related parameters are preserved in public intefaces, but they are not passed down anymore. in some places, we enforce that the host on which thrift protocol is served should be identical to that of binary. and actually, scylla always use the same host address for both thrift and binary protocols. so we replace the address like `self.network_interfaces['thrift'][0]` with `self.network_interfaces['binary'][0]`. Refs scylladb/scylladb#3811 Refs scylladb/scylladb#18416 Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>

cassandra-cli was removed in Cassandra 2.2.0. and it was replaced by cqlsh in newer releases. actually, scylla-tools does not build or package this command line tool at all. so let's drop the related helpers for accessing this tool. in a following-up change, we will drop the thrift related APIs. Refs scylladb/scylladb#3811 Refs scylladb/scylladb#18416 Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>

thrift was deprecated in both ScyllaDB and Cassandra. so there is no need to pass it around. to be compatible with existing users of the ccmlib, the thrift related parameters are preserved in public intefaces, but they are not passed down anymore. in some places, we enforce that the host on which thrift protocol is served should be identical to that of binary. and actually, scylla always use the same host address for both thrift and binary protocols. so we replace the address like `self.network_interfaces['thrift'][0]` with `self.network_interfaces['binary'][0]`. Refs scylladb/scylladb#3811 Refs scylladb/scylladb#18416 Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>

thrift support was deprecated since ScyllaDB 5.2 > Thrift API - legacy ScyllaDB (and Apache Cassandra) API is > deprecated and will be removed in followup release. Thrift has > been disabled by default. so let's drop it. in this change, * thrift protocol support is dropped * all references to thrift support in document are dropped * the "thrift_version" column in system.local table is preserved for backward compatibility, as we could load from an existing system.local table which still contains this clolumn, so we need to write this column as well. * "/storage_service/rpc_server" is only preserved for backward compatibility with java-based nodetool. Fixes scylladb#3811 Fixes scylladb#18416 Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>

…e_client_key() since we've dropped the thift support, the `client_type` is always `cql`, there is no need to differentiate different clients anymore. so, we change `make_client_key()` so that it only return the IP address and port. Refs scylladb#3811 Refs scylladb#18416 Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>

so we don't create new sstables with this unused column, but we can still open old sstables of this table which was created with the old schema. Refs scylladb#3811 Refs scylladb#18416 Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>