Skip to content

Conversation

@rmoneys
Copy link
Contributor

@rmoneys rmoneys commented Sep 15, 2022

https://synccomputing.atlassian.net/browse/PROD-429

In the case described in the ticket there are no stage, SQL or application end events in the log. There are only these events:

% jq -r .Event issues/prod-429/eventlog
DBCEventLoggingListenerMetadata
SparkListenerResourceProfileAdded
SparkListenerBlockManagerAdded
SparkListenerEnvironmentUpdate
SparkListenerApplicationStart
SparkListenerExecutorAdded
SparkListenerBlockManagerAdded

In this scenario the finish_time on the application is not set leading to the exception described in the ticket. The solution here builds on the validation that mandates end events for jobs and stages to also mandate events required to set the finish time.

Formatting removed the carriage returns so the file looks completely changed, but really only lines [26,29] were added.

A test for the solution is provided in https://github.com/synccomputingcode/sync_backend/pull/184

@rmoneys rmoneys requested a review from gorskysd September 15, 2022 06:05
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Contributor

@gorskysd gorskysd left a comment

Choose a reason for hiding this comment

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

Nice job identifying this as a lazy event! Looks good to me.

Only thing is if you want to increment the version, but I'll PR another log_parser change soon and can do it there.

@rmoneys
Copy link
Contributor Author

rmoneys commented Sep 20, 2022

Thanks @gorskysd, I'll let you increment the version.

The companion PR, https://github.com/synccomputingcode/sync_backend/pull/184, has a test to validate the new code. You had mentioned there being little expectation for tests under the internal_tools directory - is there a better place for the test?

@rmoneys rmoneys merged commit 0b1a7af into main Sep 20, 2022
@rmoneys rmoneys deleted the PROD-429-missing-app-end-events branch September 20, 2022 00:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants