From 5dca183b7b275268227fe122c54c8dd305eb7954 Mon Sep 17 00:00:00 2001 From: Matthias Date: Thu, 20 Jan 2022 19:37:01 +0100 Subject: [PATCH 1/6] Combine order and Trade migrations to better facilitate migrations in advanced DB systems --- freqtrade/persistence/migrations.py | 44 +++++++++++++++-------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/freqtrade/persistence/migrations.py b/freqtrade/persistence/migrations.py index 1839c4130..de3179a3d 100644 --- a/freqtrade/persistence/migrations.py +++ b/freqtrade/persistence/migrations.py @@ -28,7 +28,10 @@ def get_backup_name(tabs, backup_prefix: str): return table_back_name -def migrate_trades_table(decl_base, inspector, engine, table_back_name: str, cols: List): +def migrate_trades_and_orders_table( + decl_base, inspector, engine, + table_back_name: str, cols: List, + order_back_name: str): fee_open = get_column_def(cols, 'fee_open', 'fee') fee_open_cost = get_column_def(cols, 'fee_open_cost', 'null') fee_open_currency = get_column_def(cols, 'fee_open_currency', 'null') @@ -65,10 +68,14 @@ def migrate_trades_table(decl_base, inspector, engine, table_back_name: str, col # Schema migration necessary with engine.begin() as connection: connection.execute(text(f"alter table trades rename to {table_back_name}")) + with engine.begin() as connection: # drop indexes on backup table in new session for index in inspector.get_indexes(table_back_name): connection.execute(text(f"drop index {index['name']}")) + + drop_orders_table(inspector, engine, order_back_name) + # let SQLAlchemy create the schema as required decl_base.metadata.create_all(engine) @@ -103,6 +110,8 @@ def migrate_trades_table(decl_base, inspector, engine, table_back_name: str, col from {table_back_name} """)) + migrate_orders_table(decl_base, engine, order_back_name, cols) + def migrate_open_orders_to_trades(engine): with engine.begin() as connection: @@ -121,19 +130,18 @@ def migrate_open_orders_to_trades(engine): """)) -def migrate_orders_table(decl_base, inspector, engine, table_back_name: str, cols: List): - # Schema migration necessary +def drop_orders_table(inspector, engine, table_back_name: str): + # Drop and recreate orders table as backup + # This drops foreign keys, too. with engine.begin() as connection: - connection.execute(text(f"alter table orders rename to {table_back_name}")) + connection.execute(text(f"create table {table_back_name} as select * from orders")) + connection.execute(text("drop table orders")) - with engine.begin() as connection: - # drop indexes on backup table in new session - for index in inspector.get_indexes(table_back_name): - connection.execute(text(f"drop index {index['name']}")) + +def migrate_orders_table(decl_base, engine, table_back_name: str, cols: List): # let SQLAlchemy create the schema as required - decl_base.metadata.create_all(engine) with engine.begin() as connection: connection.execute(text(f""" insert into orders ( id, ft_trade_id, ft_order_side, ft_pair, ft_is_open, order_id, @@ -155,11 +163,14 @@ def check_migrate(engine, decl_base, previous_tables) -> None: cols = inspector.get_columns('trades') tabs = get_table_names_for_table(inspector, 'trades') table_back_name = get_backup_name(tabs, 'trades_bak') + order_tabs = get_table_names_for_table(inspector, 'orders') + order_table_bak_name = get_backup_name(order_tabs, 'orders_bak') - # Check for latest column - if not has_column(cols, 'buy_tag'): + # Check if migration necessary + # Migrates both trades and orders table! logger.info(f'Running database migration for trades - backup: {table_back_name}') - migrate_trades_table(decl_base, inspector, engine, table_back_name, cols) + migrate_trades_and_orders_table( + decl_base, inspector, engine, table_back_name, cols, order_table_bak_name) # Reread columns - the above recreated the table! inspector = inspect(engine) cols = inspector.get_columns('trades') @@ -167,12 +178,3 @@ def check_migrate(engine, decl_base, previous_tables) -> None: if 'orders' not in previous_tables and 'trades' in previous_tables: logger.info('Moving open orders to Orders table.') migrate_open_orders_to_trades(engine) - else: - cols_order = inspector.get_columns('orders') - - if not has_column(cols_order, 'average'): - tabs = get_table_names_for_table(inspector, 'orders') - # Empty for now - as there is only one iteration of the orders table so far. - table_back_name = get_backup_name(tabs, 'orders_bak') - - migrate_orders_table(decl_base, inspector, engine, table_back_name, cols) From 19948a6f89dde7ab3a9cc5433f0d0dfa50e96407 Mon Sep 17 00:00:00 2001 From: Matthias Date: Fri, 21 Jan 2022 06:51:04 +0100 Subject: [PATCH 2/6] Try fix sequence migrations --- freqtrade/persistence/migrations.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/freqtrade/persistence/migrations.py b/freqtrade/persistence/migrations.py index de3179a3d..82f31d7a8 100644 --- a/freqtrade/persistence/migrations.py +++ b/freqtrade/persistence/migrations.py @@ -28,6 +28,27 @@ def get_backup_name(tabs, backup_prefix: str): return table_back_name +def get_last_sequence_ids(engine, inspector): + order_id: int = None + trade_id: int = None + + if engine.name == 'postgresql': + with engine.begin() as connection: + x = connection.execute( + text("select sequencename, last_value from pg_sequences")).fetchall() + ts = [s[1]for s in x if s[0].startswith('trades_id') and s[1] is not None] + os = [s[1] for s in x if s[0].startswith('orders_id') and s[1] is not None] + trade_id = max(ts) + order_id = max(os) + + return order_id, trade_id + + +def set_sequence_ids(engine, order_id, trade_id): + + if engine.name == 'postgresql': + pass + def migrate_trades_and_orders_table( decl_base, inspector, engine, table_back_name: str, cols: List, @@ -74,6 +95,8 @@ def migrate_trades_and_orders_table( for index in inspector.get_indexes(table_back_name): connection.execute(text(f"drop index {index['name']}")) + trade_id, order_id = get_last_sequence_ids(engine, inspector) + drop_orders_table(inspector, engine, order_back_name) # let SQLAlchemy create the schema as required @@ -111,6 +134,7 @@ def migrate_trades_and_orders_table( """)) migrate_orders_table(decl_base, engine, order_back_name, cols) + set_sequence_ids(engine, order_id, trade_id) def migrate_open_orders_to_trades(engine): From c265f393239bc0dc37338b12fac149b1dca1161c Mon Sep 17 00:00:00 2001 From: Matthias Date: Fri, 21 Jan 2022 12:55:35 +0100 Subject: [PATCH 3/6] Update sequences for postgres --- freqtrade/persistence/migrations.py | 34 +++++++++++++++++------------ 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/freqtrade/persistence/migrations.py b/freqtrade/persistence/migrations.py index 82f31d7a8..e96f83caa 100644 --- a/freqtrade/persistence/migrations.py +++ b/freqtrade/persistence/migrations.py @@ -28,30 +28,35 @@ def get_backup_name(tabs, backup_prefix: str): return table_back_name -def get_last_sequence_ids(engine, inspector): +def get_last_sequence_ids(engine, trade_back_name, order_back_name): order_id: int = None trade_id: int = None if engine.name == 'postgresql': with engine.begin() as connection: - x = connection.execute( - text("select sequencename, last_value from pg_sequences")).fetchall() - ts = [s[1]for s in x if s[0].startswith('trades_id') and s[1] is not None] - os = [s[1] for s in x if s[0].startswith('orders_id') and s[1] is not None] - trade_id = max(ts) - order_id = max(os) - + trade_id = connection.execute(text("select nextval('trades_id_seq')")).fetchone()[0] + order_id = connection.execute(text("select nextval('orders_id_seq')")).fetchone()[0] + with engine.begin() as connection: + connection.execute(text( + f"ALTER SEQUENCE orders_id_seq rename to {order_back_name}_id_seq_bak")) + connection.execute(text( + f"ALTER SEQUENCE trades_id_seq rename to {trade_back_name}_id_seq_bak")) return order_id, trade_id def set_sequence_ids(engine, order_id, trade_id): if engine.name == 'postgresql': - pass + with engine.begin() as connection: + if order_id: + connection.execute(text(f"ALTER SEQUENCE orders_id_seq RESTART WITH {order_id}")) + if trade_id: + connection.execute(text(f"ALTER SEQUENCE trades_id_seq RESTART WITH {trade_id}")) + def migrate_trades_and_orders_table( decl_base, inspector, engine, - table_back_name: str, cols: List, + trade_back_name: str, cols: List, order_back_name: str): fee_open = get_column_def(cols, 'fee_open', 'fee') fee_open_cost = get_column_def(cols, 'fee_open_cost', 'null') @@ -88,14 +93,14 @@ def migrate_trades_and_orders_table( # Schema migration necessary with engine.begin() as connection: - connection.execute(text(f"alter table trades rename to {table_back_name}")) + connection.execute(text(f"alter table trades rename to {trade_back_name}")) with engine.begin() as connection: # drop indexes on backup table in new session - for index in inspector.get_indexes(table_back_name): + for index in inspector.get_indexes(trade_back_name): connection.execute(text(f"drop index {index['name']}")) - trade_id, order_id = get_last_sequence_ids(engine, inspector) + order_id, trade_id = get_last_sequence_ids(engine, trade_back_name, order_back_name) drop_orders_table(inspector, engine, order_back_name) @@ -130,7 +135,7 @@ def migrate_trades_and_orders_table( {sell_order_status} sell_order_status, {strategy} strategy, {buy_tag} buy_tag, {timeframe} timeframe, {open_trade_value} open_trade_value, {close_profit_abs} close_profit_abs - from {table_back_name} + from {trade_back_name} """)) migrate_orders_table(decl_base, engine, order_back_name, cols) @@ -192,6 +197,7 @@ def check_migrate(engine, decl_base, previous_tables) -> None: # Check if migration necessary # Migrates both trades and orders table! + if not has_column(cols, 'buy_tag'): logger.info(f'Running database migration for trades - backup: {table_back_name}') migrate_trades_and_orders_table( decl_base, inspector, engine, table_back_name, cols, order_table_bak_name) From 3d94d7df5c3c259eddcf99e3991668f2efe83193 Mon Sep 17 00:00:00 2001 From: Matthias Date: Fri, 21 Jan 2022 19:19:04 +0100 Subject: [PATCH 4/6] Update migrations for mariadb --- freqtrade/persistence/migrations.py | 8 ++++++-- tests/test_persistence.py | 5 +++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/freqtrade/persistence/migrations.py b/freqtrade/persistence/migrations.py index e96f83caa..9f7ab29cd 100644 --- a/freqtrade/persistence/migrations.py +++ b/freqtrade/persistence/migrations.py @@ -98,7 +98,10 @@ def migrate_trades_and_orders_table( with engine.begin() as connection: # drop indexes on backup table in new session for index in inspector.get_indexes(trade_back_name): - connection.execute(text(f"drop index {index['name']}")) + if engine.name == 'mysql': + connection.execute(text(f"drop index {index['name']} on {trade_back_name}")) + else: + connection.execute(text(f"drop index {index['name']}")) order_id, trade_id = get_last_sequence_ids(engine, trade_back_name, order_back_name) @@ -198,7 +201,8 @@ def check_migrate(engine, decl_base, previous_tables) -> None: # Check if migration necessary # Migrates both trades and orders table! if not has_column(cols, 'buy_tag'): - logger.info(f'Running database migration for trades - backup: {table_back_name}') + logger.info(f"Running database migration for trades - " + f"backup: {table_back_name}, {order_table_bak_name}") migrate_trades_and_orders_table( decl_base, inspector, engine, table_back_name, cols, order_table_bak_name) # Reread columns - the above recreated the table! diff --git a/tests/test_persistence.py b/tests/test_persistence.py index d98238f6f..b239f6d70 100644 --- a/tests/test_persistence.py +++ b/tests/test_persistence.py @@ -600,7 +600,8 @@ def test_migrate_new(mocker, default_conf, fee, caplog): assert trade.stoploss_last_update is None assert log_has("trying trades_bak1", caplog) assert log_has("trying trades_bak2", caplog) - assert log_has("Running database migration for trades - backup: trades_bak2", caplog) + assert log_has("Running database migration for trades - backup: trades_bak2, orders_bak0", + caplog) assert trade.open_trade_value == trade._calc_open_trade_value() assert trade.close_profit_abs is None @@ -733,7 +734,7 @@ def test_migrate_mid_state(mocker, default_conf, fee, caplog): assert trade.initial_stop_loss == 0.0 assert trade.open_trade_value == trade._calc_open_trade_value() assert log_has("trying trades_bak0", caplog) - assert log_has("Running database migration for trades - backup: trades_bak0", caplog) + assert log_has("Running database migration for trades - backup: trades_bak0, orders_bak0", caplog) def test_adjust_stop_loss(fee): From bd4014e1e67fb7e407797bffd855c9090fe6d36b Mon Sep 17 00:00:00 2001 From: Matthias Date: Fri, 21 Jan 2022 19:42:31 +0100 Subject: [PATCH 5/6] Small cleanup --- freqtrade/persistence/migrations.py | 8 ++-- tests/test_persistence.py | 64 ++--------------------------- 2 files changed, 7 insertions(+), 65 deletions(-) diff --git a/freqtrade/persistence/migrations.py b/freqtrade/persistence/migrations.py index 9f7ab29cd..60c0eb5f9 100644 --- a/freqtrade/persistence/migrations.py +++ b/freqtrade/persistence/migrations.py @@ -105,7 +105,7 @@ def migrate_trades_and_orders_table( order_id, trade_id = get_last_sequence_ids(engine, trade_back_name, order_back_name) - drop_orders_table(inspector, engine, order_back_name) + drop_orders_table(engine, order_back_name) # let SQLAlchemy create the schema as required decl_base.metadata.create_all(engine) @@ -141,7 +141,7 @@ def migrate_trades_and_orders_table( from {trade_back_name} """)) - migrate_orders_table(decl_base, engine, order_back_name, cols) + migrate_orders_table(engine, order_back_name, cols) set_sequence_ids(engine, order_id, trade_id) @@ -162,7 +162,7 @@ def migrate_open_orders_to_trades(engine): """)) -def drop_orders_table(inspector, engine, table_back_name: str): +def drop_orders_table(engine, table_back_name: str): # Drop and recreate orders table as backup # This drops foreign keys, too. @@ -171,7 +171,7 @@ def drop_orders_table(inspector, engine, table_back_name: str): connection.execute(text("drop table orders")) -def migrate_orders_table(decl_base, engine, table_back_name: str, cols: List): +def migrate_orders_table(engine, table_back_name: str, cols: List): # let SQLAlchemy create the schema as required with engine.begin() as connection: diff --git a/tests/test_persistence.py b/tests/test_persistence.py index b239f6d70..8305fe0f5 100644 --- a/tests/test_persistence.py +++ b/tests/test_persistence.py @@ -8,7 +8,7 @@ from unittest.mock import MagicMock import arrow import pytest -from sqlalchemy import create_engine, inspect, text +from sqlalchemy import create_engine, text from freqtrade import constants from freqtrade.exceptions import DependencyException, OperationalException @@ -614,65 +614,6 @@ def test_migrate_new(mocker, default_conf, fee, caplog): assert orders[1].order_id == 'stop_order_id222' assert orders[1].ft_order_side == 'stoploss' - caplog.clear() - # Drop latest column - with engine.begin() as connection: - connection.execute(text("alter table orders rename to orders_bak")) - inspector = inspect(engine) - - with engine.begin() as connection: - for index in inspector.get_indexes('orders_bak'): - connection.execute(text(f"drop index {index['name']}")) - # Recreate table - connection.execute(text(""" - CREATE TABLE orders ( - id INTEGER NOT NULL, - ft_trade_id INTEGER, - ft_order_side VARCHAR NOT NULL, - ft_pair VARCHAR NOT NULL, - ft_is_open BOOLEAN NOT NULL, - order_id VARCHAR NOT NULL, - status VARCHAR, - symbol VARCHAR, - order_type VARCHAR, - side VARCHAR, - price FLOAT, - amount FLOAT, - filled FLOAT, - remaining FLOAT, - cost FLOAT, - order_date DATETIME, - order_filled_date DATETIME, - order_update_date DATETIME, - PRIMARY KEY (id), - CONSTRAINT _order_pair_order_id UNIQUE (ft_pair, order_id), - FOREIGN KEY(ft_trade_id) REFERENCES trades (id) - ) - """)) - - connection.execute(text(""" - insert into orders ( id, ft_trade_id, ft_order_side, ft_pair, ft_is_open, order_id, status, - symbol, order_type, side, price, amount, filled, remaining, cost, order_date, - order_filled_date, order_update_date) - select id, ft_trade_id, ft_order_side, ft_pair, ft_is_open, order_id, status, - symbol, order_type, side, price, amount, filled, remaining, cost, order_date, - order_filled_date, order_update_date - from orders_bak - """)) - - # Run init to test migration - init_db(default_conf['db_url'], default_conf['dry_run']) - - assert log_has("trying orders_bak1", caplog) - - orders = Order.query.all() - assert len(orders) == 2 - assert orders[0].order_id == 'buy_order' - assert orders[0].ft_order_side == 'buy' - - assert orders[1].order_id == 'stop_order_id222' - assert orders[1].ft_order_side == 'stoploss' - def test_migrate_mid_state(mocker, default_conf, fee, caplog): """ @@ -734,7 +675,8 @@ def test_migrate_mid_state(mocker, default_conf, fee, caplog): assert trade.initial_stop_loss == 0.0 assert trade.open_trade_value == trade._calc_open_trade_value() assert log_has("trying trades_bak0", caplog) - assert log_has("Running database migration for trades - backup: trades_bak0, orders_bak0", caplog) + assert log_has("Running database migration for trades - backup: trades_bak0, orders_bak0", + caplog) def test_adjust_stop_loss(fee): From 8f2425e49f856a38a09c7a4a3ec17b5dc446b4a7 Mon Sep 17 00:00:00 2001 From: Matthias Date: Sun, 6 Feb 2022 14:06:46 +0100 Subject: [PATCH 6/6] Add rudimentary tests for pg-specific stuff --- tests/test_persistence.py | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/test_persistence.py b/tests/test_persistence.py index 8305fe0f5..df989b645 100644 --- a/tests/test_persistence.py +++ b/tests/test_persistence.py @@ -13,6 +13,7 @@ from sqlalchemy import create_engine, text from freqtrade import constants from freqtrade.exceptions import DependencyException, OperationalException from freqtrade.persistence import LocalTrade, Order, Trade, clean_dry_run_db, init_db +from freqtrade.persistence.migrations import get_last_sequence_ids, set_sequence_ids from tests.conftest import create_mock_trades, create_mock_trades_usdt, log_has, log_has_re @@ -679,6 +680,38 @@ def test_migrate_mid_state(mocker, default_conf, fee, caplog): caplog) +def test_migrate_get_last_sequence_ids(): + engine = MagicMock() + engine.begin = MagicMock() + engine.name = 'postgresql' + get_last_sequence_ids(engine, 'trades_bak', 'orders_bak') + + assert engine.begin.call_count == 2 + engine.reset_mock() + engine.begin.reset_mock() + + engine.name = 'somethingelse' + get_last_sequence_ids(engine, 'trades_bak', 'orders_bak') + + assert engine.begin.call_count == 0 + + +def test_migrate_set_sequence_ids(): + engine = MagicMock() + engine.begin = MagicMock() + engine.name = 'postgresql' + set_sequence_ids(engine, 22, 55) + + assert engine.begin.call_count == 1 + engine.reset_mock() + engine.begin.reset_mock() + + engine.name = 'somethingelse' + set_sequence_ids(engine, 22, 55) + + assert engine.begin.call_count == 0 + + def test_adjust_stop_loss(fee): trade = Trade( pair='ADA/USDT',