Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore(similarity): Add try catch around insert grouping record #1184

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

chore(similarity): Add try catch around insert grouping record #1184

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
db3a02e
chore(similarity): Add try catch around insert grouping record
jangjodi Sep 19, 2024
331fc5d
ref: Create new sessiona and use commit and rollback
jangjodi Sep 19, 2024
66d436b
ref: Remove log and use on_conflict_do_nothing
jangjodi Sep 20, 2024
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
31 changes: 24 additions & 7 deletions src/seer/grouping/grouping.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from pydantic import BaseModel, ValidationInfo, field_validator
from sentence_transformers import SentenceTransformer
from sqlalchemy.dialects.postgresql import insert
from sqlalchemy.exc import IntegrityError
from torch.cuda import OutOfMemoryError

from seer.db import DbGroupingRecord, Session
Expand Down Expand Up @@ -477,7 +478,6 @@ def insert_new_grouping_record(
) -> None:
"""
Inserts a new GroupingRecord into the database if the group_hash does not already exist.
If new grouping record was created, return the id.

:param session: The database session.
:param issue: The issue to insert as a new GroupingRecord.
Expand All @@ -489,6 +489,12 @@ def insert_new_grouping_record(
.first()
)

extra = {
"project_id": issue.project_id,
"stacktrace_length": len(issue.stacktrace),
"input_hash": issue.hash,
}

if existing_record is None:
new_record = GroupingRecord(
project_id=issue.project_id,
Expand All @@ -498,15 +504,26 @@ def insert_new_grouping_record(
error_type=issue.exception_type,
).to_db_model()
session.add(new_record)

try:
session.flush()
Copy link
Member

Choose a reason for hiding this comment

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

why flush?

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 initially thought we didn't want to rollback the entire session, but that was incorrect. I created a new session to rollback, but would it also work if I kept the session parameter and did rollback since there were no other adds?

except IntegrityError:
session.expunge(new_record)
existing_record = (
session.query(DbGroupingRecord)
.filter_by(hash=issue.hash, project_id=issue.project_id)
.first()
)
extra["existing_hash"] = existing_record.hash
logger.info(
"group_already_exists_in_seer_db",
extra=extra,
)
else:
extra["existing_hash"] = existing_record.hash
logger.info(
"group_already_exists_in_seer_db",
extra={
"existing_hash": existing_record.hash,
"project_id": issue.project_id,
"stacktrace_length": len(issue.stacktrace),
"input_hash": issue.hash,
},
extra=extra,
)

@sentry_sdk.tracing.trace
Expand Down
Loading