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

Fixes 2474: Recover from tasks panics #446

Merged
merged 1 commit into from
Nov 3, 2023

Conversation

jlsherrill
Copy link
Member

@jlsherrill jlsherrill commented Oct 26, 2023

Summary

Previously if a task paniced for some reason, the worker routine would get 'stuck' and refuse to process anything else. Now the task is marked as failed and the worker can continue to pick up another task.

Testing steps

modify the pkg/taskss/introspection.go to panic sometimes or all the time:

	if rand.Int()%2 == 0 {
		panic(fmt.Errorf("You are unlucky!!"))
	}

Create some repository with with snapshotting turned off (for simplicity)

curl -X POST --location "http://localhost:8000/api/content-sources/v1.0/repositories/" \
    -H "x-rh-identity: eyJpZGVudGl0eSI6eyJ0eXBlIjoiVXNlciIsInVzZXIiOnsidXNlcm5hbWUiOiJqdXN0aW4ifSwiYWNjb3VudF9udW1iZXIiOiIxMTQ4NjU4OSIsImludGVybmFsIjp7Im9yZ19pZCI6IjE2Nzg4NjE2In19fQo=" \
    -H "Content-Type: application/json" \
    -d "{
          \"name\": \"errata\",
          \"url\": \"https://jlsherrill.fedorapeople.org/fake-repos/needed-errata/\",
          \"snapshot\": false
        }"

Now trigger introspection:

 OPTIONS_ALWAYS_RUN_CRON_TASKS=true go run cmd/external-repos/main.go nightly-jobs

Run this a few times. eventually your workers will stop processing tasks. With this PR, they should always recover and mark the task as failed. You can check the tasks table' status column to confirm they are marked as failed.

Checklist

  • Tested with snapshotting feature disabled and pulp server URL not configured if appropriate

@jlsherrill
Copy link
Member Author

@jlsherrill
Copy link
Member Author

/retest

1 similar comment
@jlsherrill
Copy link
Member Author

/retest

Copy link
Contributor

@rverdile rverdile left a comment

Choose a reason for hiding this comment

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

nice!

@jlsherrill
Copy link
Member Author

merging as this does not require qe and the failing backend test is unrelated

@jlsherrill jlsherrill merged commit 0316cb8 into content-services:main Nov 3, 2023
7 checks passed
@jlsherrill jlsherrill deleted the 2474 branch November 3, 2023 19:51
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.

2 participants