measurement_feedback is broken

[hellais]

I believe since we made the switch to the cluster, the measurement_feedback endpoints are no longer working.

They are failing with the following error:

Feb 06 09:41:19 backend-fsn.ooni.org ooni-api[4098088]: INFO DB::Exception: Storage ReplicatedMergeTree doesn't support FINAL. Stack trace:

                                                        0. DB::Exception::Exception(DB::Exception::MessageMasked&&, int, bool) @ 0x000000000d16d15b
                                                        1. DB::Exception::Exception(PreformattedMessage&&, int) @ 0x0000000007dacdec
                                                        2. DB::Exception::Exception<String>(int, FormatStringHelperImpl<std::type_identity<String>::type>, String&&) @ 0x0000000007dac92b
                                                        3. DB::QueryAnalyzer::validateTableExpressionModifiers(std::shared_ptr<DB::IQueryTreeNode> const&, DB::IdentifierResolveScope&) @ 0x0000000010a8346d
                                                        4. DB::QueryAnalyzer::resolveQueryJoinTreeNode(std::shared_ptr<DB::IQueryTreeNode>&, DB::IdentifierResolveScope&, DB::QueryExpressionsAliasVisitor&) @ 0x0000000010aba392
                                                        5. DB::QueryAnalyzer::resolveQuery(std::shared_ptr<DB::IQueryTreeNode> const&, DB::IdentifierResolveScope&) @ 0x0000000010a7da9f
                                                        6. DB::QueryAnalyzer::resolve(std::shared_ptr<DB::IQueryTreeNode>&, std::shared_ptr<DB::IQueryTreeNode> const&, std::shared_ptr<DB::Context const>) @ 0x0000000010a7be4c
                                                        7. DB::QueryAnalysisPass::run(std::shared_ptr<DB::IQueryTreeNode>&, std::shared_ptr<DB::Context const>) @ 0x0000000010a7b705
                                                        8. DB::QueryTreePassManager::run(std::shared_ptr<DB::IQueryTreeNode>) @ 0x0000000010f845f6
                                                        9. DB::(anonymous namespace)::buildQueryTreeAndRunPasses(std::shared_ptr<DB::IAST> const&, DB::SelectQueryOptions const&, std::shared_ptr<DB::Context const> const&, std::shared_ptr<DB::IStorage> const&) (.llvm.11868937179506794133) @ 0x000000001105197d
                                                        10. DB::InterpreterSelectQueryAnalyzer::InterpreterSelectQueryAnalyzer(std::shared_ptr<DB::IAST> const&, std::shared_ptr<DB::Context const> const&, DB::SelectQueryOptions const&, std::vector<String, std::allocator<String>> const&) @ 0x000000001104f95c
                                                        11. std::unique_ptr<DB::IInterpreter, std::default_delete<DB::IInterpreter>> std::__function::__policy_invoker<std::unique_ptr<DB::IInterpreter, std::default_delete<DB::IInterpreter>> (DB::InterpreterFactory::Arguments const&)>::__call_impl<std::__function::__default_alloc_func<DB::registerInterpreterSelectQueryAnalyzer(DB::InterpreterFactory&)::$_0, std::unique_ptr<DB::IInterpreter, std::default_delete<DB::IInterpreter>> (DB::InterpreterFactory::Arguments const&)>>(std::__function::__policy_storage const*, DB::InterpreterFactory::Arguments const&) (.llvm.118689:

We should look into changing the FINAL part of those queries

[aagbsn]

It looks like the database on cluster was created using ReplicatedMergeTree instead of ReplicatedReplacingMergeTree (which supports FINAL):

1. │ CREATE TABLE ooni.msmt_feedback
(
    `measurement_uid` String,
    `account_id` String,
    `status` String,
    `update_time` DateTime64(3) MATERIALIZED now64(),
    `comment` String
)
ENGINE = ReplicatedMergeTree('/clickhouse/{cluster}/tables/ooni/msmt_feedback/{shard}', '{replica}')
ORDER BY (measurement_uid, account_id)
SETTINGS index_granularity = 4 │

see comment from code:

# # measurement feedback 

from ooniapi.database import insert_click


"""
CREATE TABLE msmt_feedback
(
    `measurement_uid` String,
    `account_id` String, 
    `status` String,
    `update_time` DateTime64(3) MATERIALIZED now64()
)
ENGINE = ReplacingMergeTree
ORDER BY (measurement_uid, account_id)
SETTINGS index_granularity = 4
""" 

If we need FINAL, consider using
https://clickhouse.com/docs/engines/table-engines/mergetree-family/replication
ReplicatedReplacingMergeTree

[aagbsn]

ooni/devops#425

I created a new table; copied the old table; and atomically renamed the tables; the old table lives at ooni.msmt_feedback_old - it keeps the metadata path
ENGINE = ReplicatedMergeTree('/clickhouse/{cluster}/tables/ooni/msmt_feedback/{shard}', '{replica}')
The new path is
ENGINE = ReplicatedReplacingMergeTree('/clickhouse/{cluster}/tables/ooni/msmt_feedback_new/{shard}', '{replica}')

This can be changed by dropping msmt_feedback_old, creating another tempnamed table with the original metadata path, and renaming it again.

[aagbsn]

CREATE TABLE ooni.msmt_feedback ON CLUSTER oonidata_cluster
(
    `measurement_uid` String,
    `account_id` String,
    `status` String,
    `update_time` DateTime64(3) DEFAULT now64(),
    `comment` String
)
ENGINE = ReplicatedReplacingMergeTree('/clickhouse/{cluster}/tables/ooni/msmt_feedback/{shard}', '{replica}')
ORDER BY (measurement_uid, account_id, update_time)
SETTINGS index_granularity = 4;

update_time is changed from MATERIALIZED so that it can be copied from the old table, but DEFAULT so it maintains the same behavior. Because there can be multiple entries for (measurement_uid, account_uid), uptime_time is added to the primary key for ReplacingMergeTree (or else an arbitrary row is dropped)