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

rgw: init oldest period after setting run_sync_thread #18664

Merged
merged 2 commits into from Nov 2, 2017

Conversation

oritwas
Copy link
Member

@oritwas oritwas commented Nov 1, 2017

Fixes: http://tracker.ceph.com/issues/21996
Signed-off-by: Orit Wasserman owasserm@redhat.com

@ceph-jenkins
Copy link
Collaborator

all commits in this PR are signed

@ceph-jenkins
Copy link
Collaborator

submodules for project are unmodified

@ceph-jenkins
Copy link
Collaborator

OK - docs built

@ceph-jenkins
Copy link
Collaborator

make check succeeded

1 similar comment
@ceph-jenkins
Copy link
Collaborator

make check succeeded

// running under radosgw-admin, so we check run_sync_thread here before
// disabling it based on the zone/zonegroup setup
meta_mgr->init_oldest_log_period();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

moving this down is fine, but the comment still talks about checking run_sync_thread here before disabling it based on the zone/zonegroup setup, which happens in the block above - could you please update or remove the comment?

Copy link
Member Author

Choose a reason for hiding this comment

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

sure

@cbodley
Copy link
Contributor

cbodley commented Nov 1, 2017

@oritwas i found the crash bug in RGWPeriodHistory - mind pulling in this fix while you're at it?

diff --git a/src/rgw/rgw_period_history.cc b/src/rgw/rgw_period_history.cc
index 895700f..1f0dbee 100644
--- a/src/rgw/rgw_period_history.cc
+++ b/src/rgw/rgw_period_history.cc
@@ -137,6 +137,8 @@ RGWPeriodHistory::Impl::Impl(CephContext* cct, Puller* puller,
 
     // get a cursor to the current period
     current_cursor = make_cursor(current_history, current_period.get_realm_epoch());
+  } else {
+    current_history = histories.end();
   }
 }

@oritwas
Copy link
Member Author

oritwas commented Nov 1, 2017

found it too :) , will include it

Fixes: http://tracker.ceph.com/issues/21996
Signed-off-by: Orit Wasserman <owasserm@redhat.com>
Fixes: http://tracker.ceph.com/issues/21996
Signed-off-by: Orit Wasserman <owasserm@redhat.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
@oritwas
Copy link
Member Author

oritwas commented Nov 1, 2017

@oritwas oritwas merged commit 87aff41 into ceph:master Nov 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants