Skip to content

Conversation

@mwooddatto
Copy link

Lots of failed jobs create lots of stack traces that just take up needless space in redis, bringing it closer to OOMing, at the worst possible time --- when we would want to diagnose failed jobs! This PR eliminates stack traces from failed jobs in qless redis, forcing the user to look in splunk, where we should be looking for such things anyway.


# TODO: pull this out into a config option.
MAX_ERROR_MESSAGE_SIZE = 10_000
MAX_ERROR_MESSAGE_SIZE = 100
Copy link

Choose a reason for hiding this comment

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

i don't remember what the AC was for that ticket but if the point is to keep just the type of error and look to splunk you could probably cut this down even more

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, this just keeps the first 100 bytes of the error message. So if the code died because of raise SomeError.new("Hello"), "Hello" would get truncated to 100 bytes.

Copy link

Choose a reason for hiding this comment

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

OH my bad I misread this as lines

Copy link

@norepIy norepIy left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@jfrancese-datto jfrancese-datto left a comment

Choose a reason for hiding this comment

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

LGTM

@jfrancese-datto
Copy link

Do we have any tests that need to get updated for this? Otherwise LGTM.

@mwooddatto mwooddatto merged commit cb1eed2 into master Nov 1, 2017
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.

4 participants