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

hammer: mon: send updated monmap to its subscribers #11457

Merged
merged 1 commit into from
Nov 29, 2016

Conversation

tchaikov
Copy link
Contributor

@tchaikov tchaikov commented Oct 13, 2016

@tchaikov tchaikov added this to the hammer milestone Oct 13, 2016
@tchaikov
Copy link
Contributor Author

will remove [DNM] once the master PR is merged.

@tchaikov tchaikov changed the title [DNM] mon: send updated monmap to its subscribers mon: send updated monmap to its subscribers Oct 25, 2016
@smithfarm smithfarm self-assigned this Nov 11, 2016
@smithfarm smithfarm changed the title mon: send updated monmap to its subscribers hammer: mon: send updated monmap to its subscribers Nov 11, 2016
@smithfarm
Copy link
Contributor

@tchaikov Could you rebase please?

@tchaikov tchaikov self-assigned this Nov 12, 2016
@smithfarm
Copy link
Contributor

smithfarm commented Nov 16, 2016

oops, this PR was not linked with the hammer backport in the tracker, so I re-did it in #11928 (closed in favor of this one)

@smithfarm
Copy link
Contributor

@tchaikov Please rebase.

prior to this change, we sends monmap when serving the subscription
requests, but the updates are not sent to the subscribers anymore.
so we need to send latest monmap to subscribes and update the
subscription status accordingly when the monmap is updated.

http://tracker.ceph.com/issues/17558
Signed-off-by: Kefu Chai <kchai@redhat.com>
(cherry picked from commit c2b348b)
Conflicts:
	src/mon/Monitor.cc: resolve conflicts
	src/mon/MonmapMonitor.cc: remove C++11-ism
@tchaikov
Copy link
Contributor Author

@smithfarm done.

smithfarm added a commit that referenced this pull request Nov 18, 2016
…bscribers

Reviewed-by: Nathan Cutler <ncutler@suse.com>
smithfarm added a commit that referenced this pull request Nov 20, 2016
…bscribers

Reviewed-by: Nathan Cutler <ncutler@suse.com>
@smithfarm
Copy link
Contributor

@gregsfortytwo This backport passed a rados suite at http://tracker.ceph.com/issues/17151#note-7 with failures that I believe have been addressed (except for http://tracker.ceph.com/issues/15139 which is caused by the build system no longer providing dumpling-era packages).

I have rebuilt the integration branch to include the two fixes and scheduled a new run at http://tracker.ceph.com/issues/17151#note-14

Do you think it's OK to merge provided the second run succeeds?

smithfarm added a commit that referenced this pull request Nov 21, 2016
…bscribers

Reviewed-by: Nathan Cutler <ncutler@suse.com>
smithfarm added a commit that referenced this pull request Nov 21, 2016
…bscribers

Reviewed-by: Nathan Cutler <ncutler@suse.com>
@smithfarm
Copy link
Contributor

@tchaikov tchaikov removed their assignment Nov 22, 2016
smithfarm added a commit that referenced this pull request Nov 23, 2016
…bscribers

Reviewed-by: Nathan Cutler <ncutler@suse.com>
smithfarm added a commit that referenced this pull request Nov 24, 2016
…bscribers

Reviewed-by: Nathan Cutler <ncutler@suse.com>
@smithfarm
Copy link
Contributor

@gregsfortytwo @athanatos This passed another rados run, see http://tracker.ceph.com/issues/17151#note-23

Do you think it can be merged?

@athanatos
Copy link
Contributor

I'm ok with this if @tchaikov is.

@tchaikov
Copy link
Contributor Author

lgtm, i vote for myself!

@smithfarm smithfarm merged commit 12d6781 into ceph:hammer Nov 29, 2016
@tchaikov tchaikov deleted the wip-17558-hammer branch November 30, 2016 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants