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

mon,osd: use GMT time for the object name of hitsets #5585

Merged
merged 5 commits into from Aug 26, 2015

Conversation

tchaikov
Copy link
Contributor

@tchaikov
Copy link
Contributor Author

was #4941

@liewegas
Copy link
Member

would it make sense to allow 'ceph osd pool set $pool use_gmt_hitset' when hit_set_count == 0?

@liewegas liewegas added this to the infernalis milestone Aug 17, 2015
@tchaikov
Copy link
Contributor Author

would it make sense to allow 'ceph osd pool set $pool use_gmt_hitset' when hit_set_count == 0?

@liewegas actually, thanks to the granularity of pg_hit_set_info_t.using_gmt, hit_set_count == 0 is not necessary, i think. we can safely trim/read the archived hit set archives using the gmt or localtime by looking at pg_hit_set_info_t.using_gmt.

@liewegas
Copy link
Member

s/gmt/utc/ ?

@tchaikov
Copy link
Contributor Author

@liewegas gmt might be more helpful for the reader, as we are basically switching to gmtime_r(3) in this change.

+  if (using_gmt) {
+    start.gmtime(ss) << "_";
+    end.gmtime(ss);
+  } else {
+    start.localtime(ss) << "_";
+    end.localtime(ss);
+  }

assert(osdmap.get_num_up_osds() == 0 ||
osdmap.get_up_osd_features() & CEPH_FEATURE_OSD_HITSET_GMT);
if (!(m->osd_features & CEPH_FEATURE_OSD_HITSET_GMT)) {
dout(0) << __func__ << " conf requires 'osd hitset gmt' but osd at "
Copy link
Member

Choose a reason for hiding this comment

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

"one or more pools uses GMT hitsets but ..." ? it's not the conf we're checking here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, will do.

@liewegas
Copy link
Member

aside from that warning msg nit, lgtm! Reviewed-by:. add needs-qa?

@tchaikov
Copy link
Contributor Author

updated the warning message as suggested by @liewegas and repushed.

@tchaikov
Copy link
Contributor Author

needs to run upgrade suite from hammer when doing cache/tiering

@tchaikov
Copy link
Contributor Author

the test added at https://github.com/ceph/ceph-qa-suite/tree/wip-tiering-hammer-x , will test it tomorrow.

@tchaikov
Copy link
Contributor Author

should reuse the CEPH_FEATURE_OSD_BITWISE_HOBJ_SORT bit, but with another name.

scratch that, CEPH_FEATURE_OSD_HITSET_GMT is reusing 1<<51[0], which is also used by CEPH_FEATURE_OSD_BITWISE_HOBJ_SORT [1].


[0] tchaikov@3893a1e#diff-33b7fd478ccf759677a0e515638a9a81R67
[1] as this pr will be merged along with 73b3ed8#diff-33b7fd478ccf759677a0e515638a9a81R67

@tchaikov
Copy link
Contributor Author

rebased against master to resolve merge conflicts and repushed.

@tchaikov
Copy link
Contributor Author

rebased & repushed

@liewegas
Copy link
Member

Final thought.. if we think there is any possibility we'll backport this feature to hammer then we should not reuse the hobj sort feature bit... @athanatos ?

@tchaikov
Copy link
Contributor Author

rebased against master to fix the build failure

@liewegas
Copy link
Member

passed my wip-sage-testing run, sage-2015-08-21_04:15:01-rados-wip-sage-testing-distro-basic-multi

* bump the encoding version of pg_hit_set_info_t to 2, so we can
  tell if the corresponding hit_set is named using localtime or
  GMT
* bump the encoding version of pg_pool_t to 20, so we can know
  if a pool is using GMT to name the hit_set archive or not. and
  we can tell if current cluster allows OSDs not support GMT
  mode or not.
* add an option named `osd_pool_use_gmt_hitset`. if enabled,
  the cluster will try to use GMT mode when creating a new pool
  if all the the up OSDs support GMT mode. if any of the
  pools in the cluster is using GMT mode, then only OSDs
  supporting GMT mode are allowed to join the cluster.

Fixes: ceph#9732
Signed-off-by: Kefu Chai <kchai@redhat.com>
allow "ceph osd pool set $pool use_gmt_hitset <true|1>" as long as
the cluster supports gmt hitset.

Fixes: ceph#9732
Signed-off-by: Kefu Chai <kchai@redhat.com>
Signed-off-by: Kefu Chai <kchai@redhat.com>
* we do not persist current hit set using get_hit_set_current_object()
  anymore, instead we always append current hitset into archive even
  !hitset.is_full(), see fbd9c15. so it's not necessary to remove the
  current hitset object before putting it to disk.

Signed-off-by: Kefu Chai <kchai@redhat.com>
Signed-off-by: Kefu Chai <kchai@redhat.com>
@tchaikov
Copy link
Contributor Author

fixed the compat version of pg_hit_set_info_t::encode(), it should be 1

tchaikov added a commit that referenced this pull request Aug 26, 2015
mon,osd: use GMT time for the object name of hitsets

Reviewed-by: Sage Weil <sage@redhat.com>
@tchaikov tchaikov merged commit 31143c7 into ceph:master Aug 26, 2015
@tchaikov tchaikov deleted the wip-4941 branch August 26, 2015 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants