Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion requirements-base.txt
Original file line number Diff line number Diff line change
Expand Up @@ -102,4 +102,4 @@ phabricator>=0.7.0
# test dependencies, but unable to move to requirements-test until
# sentry.utils.pytest and sentry.testutils are moved to tests/
selenium>=4.1.5
sqlparse>=0.2.4
sqlparse>=0.3.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes but i couldn't figure out how to add a git dependency in our requirements. We need a fix from master to get anywhere at all

2 changes: 1 addition & 1 deletion requirements-dev-frozen.txt
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ sniffio==1.2.0
snuba-sdk==1.0.0
sortedcontainers==2.4.0
soupsieve==2.3.2.post1
sqlparse==0.2.4
sqlparse==0.4.2
statsd==3.3
structlog==21.1.0
symbolic==8.7.1
Expand Down
2 changes: 1 addition & 1 deletion requirements-frozen.txt
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ sniffio==1.2.0
snuba-sdk==1.0.0
sortedcontainers==2.4.0
soupsieve==2.3.2.post1
sqlparse==0.2.4
sqlparse==0.4.2
statsd==3.3
structlog==21.1.0
symbolic==8.7.1
Expand Down
11 changes: 10 additions & 1 deletion src/sentry/conf/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,17 @@ def env(key, default="", type=None):
"PORT": "",
"AUTOCOMMIT": True,
"ATOMIC_REQUESTS": False,
}
},
"metrics-spanner": {
"ENGINE": "django_spanner",
"PROJECT": "search-and-storage",
"INSTANCE": "markus-test-spanner",
"NAME": "markus-test-spanner-db",
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you expect to move this out of the self hosted settings ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't thought about it yet. Perhaps we can move this database config into getsentry entirely again, where the db router also already exists.

}

DATABASE_ROUTERS = ("sentry.db.router.MultiDatabaseRouter",)

if "DATABASE_URL" in os.environ:
url = urlparse(os.environ["DATABASE_URL"])

Expand Down Expand Up @@ -321,6 +329,7 @@ def env(key, default="", type=None):
]

INSTALLED_APPS = (
"django_spanner",
"django.contrib.admin",
"django.contrib.auth",
"django.contrib.contenttypes",
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/db/models/fields/bounded.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class BoundedBigAutoField(models.AutoField): # type: ignore
MAX_VALUE = 9223372036854775807

def db_type(self, connection: BaseDatabaseWrapper) -> str:
return "bigserial"
return "int64"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this for ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs to be fixed. somehow this type makes it into the generated google standard sql statement without any sort of translation. So we need to either make db_type check the database backend somehow, or stop using our own bigautofield.


def get_related_db_type(self, connection: BaseDatabaseWrapper) -> str:
return cast(str, BoundedBigIntegerField().db_type(connection))
Expand Down
60 changes: 60 additions & 0 deletions src/sentry/db/router.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
_db_table_to_db = {
"sentry_stringindexer": "metrics-spanner", # will replace ^ table above
}


def db_for_model(model):
return db_for_table(model._meta.db_table)


def db_for_table(table):
return _db_table_to_db.get(table, "default")


class MultiDatabaseRouter:
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self._setup_replicas()

def _setup_replicas(self):
from django.conf import settings

self._replicas = {}
for db, db_conf in settings.DATABASES.items():
primary = db_conf.get("REPLICA_OF")
if primary:
self._replicas[primary] = db

def db_for_read(self, model, **hints):
db = db_for_model(model)
if hints.get("replica", False):
db = self._replicas.get(db, db)
return db

def db_for_write(self, model, **hints):
return db_for_model(model)

def allow_relation(self, obj1, obj2, **hints):
return db_for_model(obj1) == db_for_model(obj2)

def allow_syncdb(self, db, model):
if db_for_model(model) == db:
return True
return False

def allow_migrate(self, db, app_label, model=None, **hints):
if model:
return db_for_model(model) == db

# We use this hint in our RunSql/RunPython migrations to help resolve databases.
if "tables" in hints:
dbs = {db_for_table(table) for table in hints["tables"]}
if len(dbs) > 1:
raise RuntimeError(
"Migration tables resolve to multiple databases. "
f"Got {dbs} when only one database should be used."
)
return dbs.pop() == db
# Assume migrations with no model routing or hints need to run on
# the default database.
return db == "default"
14 changes: 1 addition & 13 deletions src/sentry/migrations/0284_metrics_indexer_alter_seq.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,4 @@ class Migration(CheckedMigration):
("sentry", "0283_extend_externalissue_key"),
]

operations = [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

raw sql statements mentioning sequence don't work. We cannot run this migration as-is, so I think we would need to edit it to skip doing that for spanner, or create a new table name etc for spanner where this migration doesn't run.

migrations.RunSQL(
"""
ALTER SEQUENCE sentry_stringindexer_id_seq START WITH 65536;
ALTER SEQUENCE sentry_stringindexer_id_seq RESTART;
""",
hints={"tables": ["sentry_stringindexer"]},
reverse_sql="""
ALTER SEQUENCE sentry_stringindexer_id_seq START WITH 1;
ALTER SEQUENCE sentry_stringindexer_id_seq RESTART;
""",
)
]
operations = []
11 changes: 0 additions & 11 deletions src/sentry/migrations/0291_add_new_perf_indexer.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,4 @@ class Migration(CheckedMigration):
fields=("string", "organization_id"), name="perf_unique_org_string"
),
),
migrations.RunSQL(
"""
ALTER SEQUENCE sentry_perfstringindexer_id_seq START WITH 65536;
ALTER SEQUENCE sentry_perfstringindexer_id_seq RESTART;
""",
hints={"tables": ["sentry_perfstringindexer"]},
reverse_sql="""
ALTER SEQUENCE sentry_perfstringindexer_id_seq START WITH 1;
ALTER SEQUENCE sentry_perfstringindexer_id_seq RESTART;
""",
),
]
3 changes: 0 additions & 3 deletions src/sentry/models/counter.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,3 @@ def create_counter_function(app_config, using, **kwargs):
)
finally:
cursor.close()


post_migrate.connect(create_counter_function, dispatch_uid="create_counter_function", weak=False)