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.

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

chore(router): provide more debug info when detecting an illegal job sequence #3787

Merged
merged 1 commit into from
Aug 25, 2023

Conversation

atzoum
Copy link
Contributor

@atzoum atzoum commented Aug 25, 2023

Description

Adding information about sequence of job statuses recorded in database to help debug job sequence issues.

Linear Ticket

Link

Security

  • The code changed/added as part of this pull request won't create any security issues with how the software is being used.

@codecov
Copy link

codecov bot commented Aug 25, 2023

Codecov Report

Patch coverage: 48.88% and project coverage change: +0.05% 🎉

Comparison is base (4e5b6a7) 69.04% compared to head (888e342) 69.09%.
Report is 1 commits behind head on master.

❗ Current head 888e342 differs from pull request most recent head 9fe5ecb. Consider uploading reports for the commit 9fe5ecb to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3787      +/-   ##
==========================================
+ Coverage   69.04%   69.09%   +0.05%     
==========================================
  Files         347      347              
  Lines       51692    51734      +42     
==========================================
+ Hits        35691    35746      +55     
+ Misses      13709    13688      -21     
- Partials     2292     2300       +8     
Files Changed Coverage Δ
router/internal/eventorder/eventorder.go 91.92% <42.10%> (-4.21%) ⬇️
router/handle_observability.go 79.23% <45.45%> (+32.93%) ⬆️
router/misc.go 75.92% <100.00%> (+1.41%) ⬆️
router/partition_worker.go 100.00% <100.00%> (ø)

... and 7 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@atzoum atzoum force-pushed the chore.userOrdering branch 4 times, most recently from a907083 to 01ef697 Compare August 25, 2023 13:47
Comment on lines +156 to +160
defer func() {
if r := recover(); r != nil {
res = fmt.Sprintf("panic in EventOrderDebugInfo: %v", r)
}
}()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this going to catch the panic and not propagate it anymore? So the next time we got an out of order rudder-server won't crash? I thought you would save the data and re-issue the panic.

Copy link
Collaborator

Choose a reason for hiding this comment

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

My bad, this is for the local panic.

@Sidddddarth Sidddddarth merged commit 9f1c5fb into master Aug 25, 2023
33 checks passed
@Sidddddarth Sidddddarth deleted the chore.userOrdering branch August 25, 2023 14:49
achettyiitr pushed a commit that referenced this pull request Aug 28, 2023
…sequence (#3787)

Adding information about sequence of job statuses recorded in database to help debug job sequence issues.
achettyiitr pushed a commit that referenced this pull request Aug 28, 2023
…sequence (#3787)

Adding information about sequence of job statuses recorded in database to help debug job sequence issues.
fracasula pushed a commit that referenced this pull request Sep 12, 2023
* chore(router): provide more debug info when detecting an illegal job sequence (#3787)

Adding information about sequence of job statuses recorded in database to help debug job sequence issues.

* chore: some more changes

* chore: fix test

* chore: fix test

* chore: fix test

* chore: some more changes

* chore: tests fix

---------

Co-authored-by: Aris Tzoumas <atzoumas@rudderstack.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants