feat(migrations): replace discover_dist with discover_dist_new

[dbanda]

This is a follow up to #5788

In this migration we replace discover_dist table with discover_dist_new that references discover_local. This makes our SaaS environments and others consistent with the self-hosted one.

[github-actions]

This PR has a migration; here is the generated SQL

-- start migrations

-- forward migration discover : 0008_discover_fix_new_dist_table
Local op: CREATE TABLE IF NOT EXISTS discover_local (event_id UUID, project_id UInt64, type LowCardinality(String), timestamp DateTime, platform LowCardinality(String), environment LowCardinality(Nullable(String)), release LowCardinality(Nullable(String)), dist LowCardinality(Nullable(String)), transaction_name LowCardinality(String), message String, title String, user String, user_hash UInt64, user_id Nullable(String), user_name Nullable(String), user_email Nullable(String), ip_address_v4 Nullable(IPv4), ip_address_v6 Nullable(IPv6), sdk_name LowCardinality(Nullable(String)), sdk_version LowCardinality(Nullable(String)), http_method LowCardinality(Nullable(String)), http_referer Nullable(String), tags Nested(key String, value String), _tags_hash_map Array(UInt64), contexts Nested(key String, value String), span_id Nullable(UInt64), trace_id Nullable(UUID), deleted UInt8) ENGINE Merge(currentDatabase(), '^errors_local$|^transactions_local$');
Distributed op: CREATE TABLE IF NOT EXISTS discover_dist_new (event_id UUID, project_id UInt64, type LowCardinality(String), timestamp DateTime, platform LowCardinality(String), environment LowCardinality(Nullable(String)), release LowCardinality(Nullable(String)), dist LowCardinality(Nullable(String)), transaction_name LowCardinality(String), message String, title String, user String, user_hash UInt64, user_id Nullable(String), user_name Nullable(String), user_email Nullable(String), ip_address_v4 Nullable(IPv4), ip_address_v6 Nullable(IPv6), sdk_name LowCardinality(Nullable(String)), sdk_version LowCardinality(Nullable(String)), http_method LowCardinality(Nullable(String)), http_referer Nullable(String), tags Nested(key String, value String), _tags_hash_map Array(UInt64), contexts Nested(key String, value String), span_id Nullable(UInt64), trace_id Nullable(UUID), deleted UInt8) ENGINE Distributed(`cluster_one_sh`, default, discover_local);
-- end forward migration discover : 0008_discover_fix_new_dist_table




-- backward migration discover : 0008_discover_fix_new_dist_table
Distributed op: DROP TABLE IF EXISTS discover_dist_new;
Local op: DROP TABLE IF EXISTS discover_local;
-- end backward migration discover : 0008_discover_fix_new_dist_table
-- forward migration discover : 0009_discover_fix_rename_dist_table
Distributed op: RENAME TABLE discover_dist TO discover_dist_temp;
Distributed op: RENAME TABLE discover_dist_new TO discover_dist;
Distributed op: DROP TABLE IF EXISTS discover_dist_temp;
-- end forward migration discover : 0009_discover_fix_rename_dist_table




-- backward migration discover : 0009_discover_fix_rename_dist_table
-- end backward migration discover : 0009_discover_fix_rename_dist_table

[codecov]

Test Failures Detected: Due to failing tests, we cannot provide coverage reports at this time.

❌ Failed Test Results:

Completed 1174 tests with 1 failed, 1171 passed and 2 skipped.

View the full list of failed tests

Test Description

Failure message

Testsuite: pytest Test name: tests.migrations.test_runner::test_reverse_all Envs: - default

Traceback (most recent call last): File ".../snuba/clickhouse/native.py", line 200, in execute result_data = query_execute() ^^^^^^^^^^^^^^^ File ".../snuba/clickhouse/native.py", line 183, in query_execute return conn.execute( # type: ignore ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File ".../local/lib/python3.11.............../site-packages/clickhouse_driver/client.py", line 373, in execute rv = self.process_ordinary_query( ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File ".../local/lib/python3.11.............../site-packages/clickhouse_driver/client.py", line 571, in process_ordinary_query return self.receive_result(with_column_types=with_column_types, ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File ".../local/lib/python3.11.............../site-packages/clickhouse_driver/client.py", line 204, in receive_result return result.get_result() ^^^^^^^^^^^^^^^^^^^ File ".../local/lib/python3.11.../site-packages/clickhouse_driver/result.py", line 50, in get_result for packet in self.packet_generator: File ".../local/lib/python3.11.............../site-packages/clickhouse_driver/client.py", line 220, in packet_generator packet = self.receive_packet() ^^^^^^^^^^^^^^^^^^^^^ File ".../local/lib/python3.11.............../site-packages/clickhouse_driver/client.py", line 237, in receive_packet raise packet.exception clickhouse_driver.errors.ServerException: Code: 60. DB::Exception: Table default.discover_local doesn't exist. Stack trace: 0. DB::Exception::Exception(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, int, bool) @ 0x8fe7c9a in ..................................................................................../usr/bin/clickhouse 1. DB::Exception::Exception<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >(int, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >&&) @ 0x90feca3 in ..................................................................................../usr/bin/clickhouse 2. void std::__1::__optional_storage_base<DB::Exception, false>::__construct<int const&, char const (&) [23], std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >(int const&, char const (&) [23], std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >&&) @ 0x10115a31 in ..................................................................................../usr/bin/clickhouse 3. DB::DatabaseCatalog::getTableImpl(DB::StorageID const&, std::__1::shared_ptr<DB::Context const>, std::__1::optional<DB::Exception>) const @ 0x10107aca in ..................................................................................../usr/bin/clickhouse 4. DB::DatabaseCatalog::getTable(DB::StorageID const&, std::__1::shared_ptr<DB::Context const>) const @ 0x1010e188 in ..................................................................................../usr/bin/clickhouse 5. DB::InterpreterAlterQuery::execute() @ 0x10271539 in ..................................................................................../usr/bin/clickhouse 6. & @ 0x10824984 in ..................................................................................../usr/bin/clickhouse 7. DB::executeQuery(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::shared_ptr<DB::Context>, bool, DB::QueryProcessingStage::Enum, bool) @ 0x10823023 in ..................................................................................../usr/bin/clickhouse 8. DB::TCPHandler::runImpl() @ 0x110c9fde in ..................................................................................../usr/bin/clickhouse 9. DB::TCPHandler::run() @ 0x110dcf79 in ..................................................................................../usr/bin/clickhouse 10. Poco::Net::TCPServerConnection::start() @ 0x13c5614f in ..................................................................................../usr/bin/clickhouse 11. Poco::Net::TCPServerDispatcher::run() @ 0x13c57bda in ..................................................................................../usr/bin/clickhouse 12. Poco::PooledThread::run() @ 0x13d89e59 in ..................................................................................../usr/bin/clickhouse 13. Poco::ThreadImpl::runnableEntry(void) @ 0x13d860ea in ..................................................................................../usr/bin/clickhouse 14. start_thread @ 0x9609 in .../lib/x86_64-linux-gnu/libpthread-2.31.so 15. clone @ 0x122293 in .../lib/x86_64-linux-gnu/libc-2.31.so The above exception was the direct cause of the following exception: Traceback (most recent call last): File ".../tests/migrations/test_runner.py", line 308, in test_reverse_all runner.reverse_migration(migration, force=True) File ".../snuba/migrations/runner.py", line 344, in reverse_migration self._reverse_migration_impl(migration_key) File ".../snuba/migrations/runner.py", line 401, in _reverse_migration_impl migration.backwards(context, dry_run) File ".../snuba/migrations/migration.py", line 193, in backwards op.execute() File ".../snuba/migrations/operations.py", line 74, in execute connection.execute(self.format_sql(), settings=self._settings) File ".../snuba/clickhouse/native.py", line 283, in execute raise ClickhouseError(e.message, code=e.code) from e snuba.clickhouse.errors.ClickhouseError: DB::Exception: Table default.discover_local doesn't exist. Stack trace: 0. DB::Exception::Exception(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, int, bool) @ 0x8fe7c9a in ..................................................................................../usr/bin/clickhouse 1. DB::Exception::Exception<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >(int, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >&&) @ 0x90feca3 in ..................................................................................../usr/bin/clickhouse 2. void std::__1::__optional_storage_base<DB::Exception, false>::__construct<int const&, char const (&) [23], std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >(int const&, char const (&) [23], std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >&&) @ 0x10115a31 in ..................................................................................../usr/bin/clickhouse 3. DB::DatabaseCatalog::getTableImpl(DB::StorageID const&, std::__1::shared_ptr<DB::Context const>, std::__1::optional<DB::Exception>) const @ 0x10107aca in ..................................................................................../usr/bin/clickhouse 4. DB::DatabaseCatalog::getTable(DB::StorageID const&, std::__1::shared_ptr<DB::Context const>) const @ 0x1010e188 in ..................................................................................../usr/bin/clickhouse 5. DB::InterpreterAlterQuery::execute() @ 0x10271539 in ..................................................................................../usr/bin/clickhouse 6. & @ 0x10824984 in ..................................................................................../usr/bin/clickhouse 7. DB::executeQuery(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::shared_ptr<DB::Context>, bool, DB::QueryProcessingStage::Enum, bool) @ 0x10823023 in ..................................................................................../usr/bin/clickhouse 8. DB::TCPHandler::runImpl() @ 0x110c9fde in ..................................................................................../usr/bin/clickhouse 9. DB::TCPHandler::run() @ 0x110dcf79 in ..................................................................................../usr/bin/clickhouse 10. Poco::Net::TCPServerConnection::start() @ 0x13c5614f in ..................................................................................../usr/bin/clickhouse 11. Poco::Net::TCPServerDispatcher::run() @ 0x13c57bda in ..................................................................................../usr/bin/clickhouse 12. Poco::PooledThread::run() @ 0x13d89e59 in ..................................................................................../usr/bin/clickhouse 13. Poco::ThreadImpl::runnableEntry(void) @ 0x13d860ea in ..................................................................................../usr/bin/clickhouse 14. start_thread @ 0x9609 in .../lib/x86_64-linux-gnu/libpthread-2.31.so 15. clone @ 0x122293 in .../lib/x86_64-linux-gnu/libc-2.31.so

[MeredithAnya]

@dbanda wait I thought self-hosted was the only thing we wanted to fix, since ST and SaaS actually have the same setup ? Seems less disruptive to change self-hosted that to change SaaS and ST?

[dbanda]

@dbanda wait I thought self-hosted was the only thing we wanted to fix, since ST and SaaS actually have the same setup ? Seems less disruptive to change self-hosted that to change SaaS and ST?

Closing this PR in favor of #5788 . I think changing self-hosted will be harder because the convention we use is that each storage has local and dist tables. If we drop the local table, we will not be able to run snuba on a single node without adding in extra code to make exceptions for discover which seems quite messy. I opted to instead just add discover local in SaaS and ST. The table wont be used, but it will just be there to keep things consistent.