Bugfix to deal with lost tasks after bad transfers #331
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If the task list also contained non transfer tasks
Less global state. Appending results for the transfer daemon to results from 1st round.
wcawijngaards
approved these changes
May 21, 2024
There was a problem hiding this comment.
The code looks nice. After reload step one, the xfrd->reload_pid variable is not assigned to a new value, where it would be set to the new reload process value in current code. After the reload completes the second step, the xfrd->reload_pid has a value, and that is fine. But just after step one it points to a deleted process, I think.
| @@ -0,0 +1,18 @@ | |||
| BaseName: tasks_after_bad_xfr | |||
| Version: 1.0 | |||
| Description: Catalog zones testing | |||
There was a problem hiding this comment.
Copy and paste mistake.
Suggested change
| Description: Catalog zones testing | |
| Description: Test tasks after bad transfers |
It would be a good idea to merge squash some commits, because a lot of earlier changes are just undone.
|
Sorry @wcawijngaards and @k0ekk0ek if you already reviewed. I think this can be very much simplified. A new (better) proposal is in PR #332 . |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The issue is that updating the database from transfers and other tasks that change the database (such as adding zones or changing patterns) are processed in so called task list. Especially applying transfers are executed batch-wise periodically as configured by
xfrd-reload-timeout:(default every second). If a zone is added it is appended to the tasklist. If there would have been an "apply transfer task" before that other (adding a zone) task, then that will be executed first. If the "apply transfer task" fails, the reload will be aborted (because the database could be corrupt) and the effect of any tasks before or after the transfer are undone. This can lead to a situation that the transfer daemon (xfrd) thinks a zone is configured, but the zone is not configured in the server process (main).