scylladb / scylladb

NoSQL data store using the seastar framework, compatible with Apache Cassandra
http://scylladb.com
GNU Affero General Public License v3.0
12.99k stars 1.24k forks source link

column_family/major_compaction/{name} either not working or has incorrect documentation #4058

Open denesb opened 5 years ago

denesb commented 5 years ago
-> http://172.17.0.3:10000/column_family/name
=> [
  "system:paxos",
  "system:batchlog",
  "system:compactions_in_progress",
  "system_auth:role_members",
  "system:hints",
  "system_schema:columns",
  "scylla_bench:test_counters",
  "system_schema:types",
  "system_distributed:view_build_status",
  "system_schema:indexes",
  "system_schema:tables",
  "system_schema:view_virtual_columns",
  "system_schema:keyspaces",
  "system_schema:scylla_tables",
  "scylla_bench:test",
  "system_schema:views",
  "system:built_views",
  "system:scylla_views_builds_in_progress",
  "system_schema:dropped_columns",
  "system_traces:node_slow_log",
  "system:IndexInfo",
  "system_schema:triggers",
  "system:local",
  "system:compaction_history",
  "system:peer_events",
  "system:peers",
  "system_traces:sessions",
  "system_schema:aggregates",
  "system:range_xfers",
  "system:sstable_activity",
  "system_schema:functions",
  "system:size_estimates",
  "system_traces:sessions_time_idx",
  "system:large_partitions",
  "system:views_builds_in_progress",
  "system_traces:node_slow_log_time_idx",
  "system_auth:roles",
  "system_traces:events"
]
-> http://172.17.0.3:10000/column_family/major_compaction/scylla_bench%3Atest
=> {
  "message": "Not found",
  "code": 404
}

Trying:

-> http://172.17.0.3:10000/column_family/major_compaction/scylla_bench:test
-> http://172.17.0.3:10000/column_family/major_compaction/scylla_bench.test

Yields the same result. The documentation says that the {name} parameters should be:

The column family name in keyspace:name format

Either the documentation is incorrect (another format is expected) or the implementation is not correct.

/cc @amnonh

denesb commented 5 years ago

I was using the Swagger UI (http://172.17.0.3:10000/ui/#!/column95family/force_major_compaction).

amnonh commented 5 years ago

I've checked, compaction is not implemented in column_family, compaction is implemented in the POST /storage_service/keyspace_compaction/{keyspace} you can specify one or more column families for a CF level compaction

denesb commented 5 years ago

So the endpoint doesn't even exists?

amnonh commented 5 years ago

the endpoint is not implemented

nyh commented 5 years ago

@amnonh there are multiple problems here I don't understand:

First, in scylla-jmx, forceMajorCompaction(boolean splitOutput) of scylla-jmx passes the splitOutput boolean as a "value" parameter to the column_family/major_compaction/ REST API. So apparently this API must exist for "nodetool compact" to work, no? But you're saying this API doesn't exist, so how does "nodetool compact" work?

Second, column_family.json says there is a column_family/major_compaction/ operation. This is what the OP found. It should be listed if it doesn't exist. Moreover, it is listed as taking a parameter called "split_output", not "value" as the scylla-jmx sends it!

And third, I can't find the implementation of the coulmn_family/major_compaction/, now you suggest it doesn't exist?

nyh commented 5 years ago

@amnonh clarified for me what's going on. Nobody uses "coulmn_family/major_compaction/'". The relevant JMX call from "nodetool compact" doesn't call this REST API, it calls /storage_service/keyspace_compaction/{keyspace}.

So if I understand correctly I think we should remove the listing of /column_family/major_compaction/ from column_family.json so people won't find it and try to use it. We could also remove the scylla-jmx code which pretends to call /column_family/major_compaction/ but never gets used by nodetool anyway...

slivne commented 5 years ago

@amnonh ping

slivne commented 5 years ago

We have many endpoints unimplemented and unfortunatly this is not documented.

Users should not use the API.

All of this should be resolved as API 2.0 will be introduced which every endpoint will be implemented.

denesb commented 5 years ago

So should I close this?

amnonh commented 4 years ago

@slivne let's close this

slivne commented 4 years ago

@amnonh rethinking this...

amnonh commented 4 years ago

I think that we should move towards v2, and leave v1 align with the jmx

nyh commented 4 years ago

@amnonh but why not remove this unimplemented API function? Nobody could use it anyway so it's not like we're breaking compatibility?

amnonh commented 4 years ago

The current API is for internal use and suppose to be one-to-one matching to the jmx.

When we find a new function we need, we only need to replace its implementation and everything else works.

nyh commented 4 weeks ago

@amnonh but why not remove this unimplemented API function? Nobody could use it anyway so it's not like we're breaking compatibility?

Maybe it's time to do this? Why are we holding on to these dead REST API functions that now that more and more people use the REST API (JMX is officially defunct), people will be tempted to try?

denesb commented 4 weeks ago

Yes, we have many such API from a time when we tried to mirror the JMX API in our REST one. It is time to remove the unimplemented ones, because no one will implement them, the agreed-upon future direction is a redesigned API (v2), not implementing he remaining endpoints that nobody uses.