rbd-mirror: configuration overrides for hard coded timers #11840
Conversation
dillaman
changed the title
| @@ -1294,6 +1294,10 @@ OPTION(rbd_mirror_journal_poll_age, OPT_DOUBLE, 5) // maximum age (in seconds) b | |||
| OPTION(rbd_mirror_journal_max_fetch_bytes, OPT_U32, 32768) // maximum bytes to read from each journal data object per fetch | |||
| OPTION(rbd_mirror_sync_point_update_age, OPT_DOUBLE, 30) // number of seconds between each update of the image sync point object number | |||
| OPTION(rbd_mirror_concurrent_image_syncs, OPT_U32, 5) // maximum number of image syncs in parallel | |||
| OPTION(rbd_mirror_update_replayers_interval, OPT_INT, 30) // interval to update replayers in rbd-mirror daemon | |||
| OPTION(rbd_mirror_deleter_retry_interval, OPT_DOUBLE, 30) // interval to check and retry the failed requests in deleter | |||
There was a problem hiding this comment.
Minor: can you rename rbd_mirror_deleter_retry_interval to rbd_mirror_delete_retry_interval
| @@ -1294,6 +1294,10 @@ OPTION(rbd_mirror_journal_poll_age, OPT_DOUBLE, 5) // maximum age (in seconds) b | |||
| OPTION(rbd_mirror_journal_max_fetch_bytes, OPT_U32, 32768) // maximum bytes to read from each journal data object per fetch | |||
| OPTION(rbd_mirror_sync_point_update_age, OPT_DOUBLE, 30) // number of seconds between each update of the image sync point object number | |||
| OPTION(rbd_mirror_concurrent_image_syncs, OPT_U32, 5) // maximum number of image syncs in parallel | |||
| OPTION(rbd_mirror_update_replayers_interval, OPT_INT, 30) // interval to update replayers in rbd-mirror daemon | |||
| OPTION(rbd_mirror_deleter_retry_interval, OPT_DOUBLE, 30) // interval to check and retry the failed requests in deleter | |||
| OPTION(rbd_mirror_pool_watcher_refresh_images_interval, OPT_INT, 30) // interval to refresh images in pool watcher | |||
There was a problem hiding this comment.
Minor: suggest something like rbd_mirror_image_directory_refresh_interval
| @@ -1294,6 +1294,10 @@ OPTION(rbd_mirror_journal_poll_age, OPT_DOUBLE, 5) // maximum age (in seconds) b | |||
| OPTION(rbd_mirror_journal_max_fetch_bytes, OPT_U32, 32768) // maximum bytes to read from each journal data object per fetch | |||
| OPTION(rbd_mirror_sync_point_update_age, OPT_DOUBLE, 30) // number of seconds between each update of the image sync point object number | |||
| OPTION(rbd_mirror_concurrent_image_syncs, OPT_U32, 5) // maximum number of image syncs in parallel | |||
| OPTION(rbd_mirror_update_replayers_interval, OPT_INT, 30) // interval to update replayers in rbd-mirror daemon | |||
There was a problem hiding this comment.
Minor: "replayers" isn't very descriptive to the function served. Suggest something like rbd_mirror_pool_refresh_interval
There was a problem hiding this comment.
Hum.... IMO, update_replayers is working for a cluster, from what I read, I think this option is used as an interval between calls of update_replayers() in Mirror.run(). And the update_replayers() is going to start or stop replayer for each peer. What about rbd_mirror_refresh_peers_interval ?
There was a problem hiding this comment.
It's refreshing the Replayers for each pool -- so I can see how rbd_mirror_pool_refresh_interval might not be exact enough. Perhaps rbd_mirror_pool_replayer_refresh_interval?
There was a problem hiding this comment.
rbd_mirror_pool_replayer_refresh_interval might be misunderstood as running a refresh() in a pool replayer. But actually it's used to refresh m_replayers in Mirror class. What about rbd_mirror_refresh_replayers_interval ?
There was a problem hiding this comment.
Longer term planning, the current Replayer will most likely be renamed to PoolReplayer and refactored so that it handles all peers for a single conceptual pool. This will allow N-way replication since the ImageReplayer would need to know all possible remote peer sources for the image so that it can pick the current primary version.
There was a problem hiding this comment.
Yes, I see that replayer is working on one pool,
but the rbd_mirror_pool_replayer_refresh_interval would be thought as the interval at this line:
https://github.com/ceph/ceph/blob/master/src/tools/rbd_mirror/PoolWatcher.cc#L78
But actually, this option is used at this line:
https://github.com/ceph/ceph/blob/master/src/tools/rbd_mirror/Mirror.cc#L240
This option means the interval between different calls of update_replayers() in Mirror.cc.
There was a problem hiding this comment.
The first one is covered by rbd_mirror_pool_watcher_refresh_images_interval (how often to scan for new images in a pool) whereas this one is for how often to refresh the (pool) replayers.
There was a problem hiding this comment.
yes, that's true, but ..... maybe I did not express myself clearly. forget it.
So what about rbd_mirror_pool_replayers_refresh_interval? use replayers rather than replayer in your suggestion.
| OPTION(rbd_mirror_update_replayers_interval, OPT_INT, 30) // interval to update replayers in rbd-mirror daemon | ||
| OPTION(rbd_mirror_deleter_retry_interval, OPT_DOUBLE, 30) // interval to check and retry the failed requests in deleter | ||
| OPTION(rbd_mirror_pool_watcher_refresh_images_interval, OPT_INT, 30) // interval to refresh images in pool watcher | ||
| OPTION(rbd_mirror_replayer_set_sources_interval, OPT_INT, 30) // interval to get images from pool watcher and set sources in replayer |
There was a problem hiding this comment.
Minor: set_sources is an implementation detail. Suggest something more like rbd_mirror_image_state_check_interval
Signed-off-by: Dongsheng Yang <dongsheng.yang@easystack.cn>
Signed-off-by: Dongsheng Yang <dongsheng.yang@easystack.cn>
yangdongsheng
force-pushed
the
rbd_mirror_options
branch
from
58c3820
to
a0fccea
Compare
|
lgtm |
No description provided.