-
Notifications
You must be signed in to change notification settings - Fork 158
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
Add --feed-lock-timeout option #1472
Add --feed-lock-timeout option #1472
Conversation
This new option sets a number of seconds to retry for if the feed is locked in contexts (like migration or rebuilds) that do not retry on their own (like automatic syncs). This is intended to help automated upgrades of GVM modules where ospd-openvas needs to be restarted, which locks the feed while the VTs are reloaded.
5410313
to
8e080ee
Compare
\fB--slave-commit-size=\fINUMBER\fB\f1 | ||
During slave updates, commit after every NUMBER updated results and hosts, 0 for unlimited. | ||
.TP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Has this been removed by accident or on purpose? Maybe it belongs to another PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This option is supposed to be removed. I think it was added back in by accident in a backport or it was just forgotten before.
@@ -6345,7 +6419,7 @@ gvm_migrate_secinfo (int feed_type) | |||
return -1; | |||
} | |||
|
|||
ret = feed_lockfile_lock (&lockfile); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about the old function? Is is still used or can it be removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The old function is still used in the automatic sync that has essentially its own retry loop.
I've also changed feed_lockfile_lock_timeout
to use it now so writing the timestamp doesn't have to be duplicated.
src/manage.c
Outdated
ret = lockfile_lock_path_nb (lockfile, get_feed_lock_path ()); | ||
if (ret == 1 && timeout_end > time (NULL)) | ||
{ | ||
if (log_timeout) | ||
{ | ||
log_timeout = FALSE; | ||
g_message ("%s: Feed is currently locked by another process," | ||
" will retry until %s.", | ||
__func__, iso_time (&timeout_end)); | ||
} | ||
gvm_sleep (1); | ||
} | ||
else if (ret) | ||
{ | ||
return ret; | ||
} | ||
} while (ret); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the meaning of ret here? 1 == locked? If yes maybe you could rename it to somthing like is_locked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've renamed the variable and added a few comments about the conditions.
src/gvmd.c
Outdated
&feed_lock_timeout, | ||
"Sets the number of seconds to retry for if the feed is locked" | ||
" in contexts (like migration or rebuilds) that do not retry" | ||
" on their own (like automatic syncs). Defaults to 0.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would explain the meaning of 0 too (i guess it is nowait).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated the text to mention that the default 0 means there is no retry.
The feed_lockfile_lock function is used to avoid duplicating the code to write the timestamp. Also, the return value checks are clarified.
The options help and man page now mention that the value 0 means no retry.
Add --feed-lock-timeout option (bp #1472)
@Mergifyio refresh |
@Mergifyio backport gvmd-20.08 |
Command
|
Command
|
This fixes the changelog by putting the entry in the 20.08.2 section and adds the slave-commit-size option back to the man page.
Add --feed-lock-timeout option (bp #1472)
What:
This new option sets a number of seconds to retry for if the feed is
locked in contexts (like migration or rebuilds) that do not retry on
their own (like automatic syncs).
Why:
This is intended to help automated upgrades of GVM modules where
ospd-openvas needs to be restarted, which locks the feed while the
VTs are reloaded.
How did you test it:
By doing the following:
gvmd --rebuild-scap
- this should fail immediately.gvmd --rebuild-scap --feed-lock-timeout 15
with the lock still active - this should generate a gvmd log message "Feed is currently locked..." and fail about 15 seconds later.gvmd --rebuild-scap --feed-lock-timeout 15
with the lock still active and releasing the lock while gvmd is waiting - this should leave a "Feed is currently locked..." gvmd log message and start the rebuild once the lock is released.gvmd --rebuild-scap --feed-lock-timeout 15
without locking - this should start the rebuild immediately without a "Feed is currently locked..." log message.Checklist: