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

Jump to bottom

nautilus: msg/simple: reset in_seq_acked to zero when session is reset #29592

Merged
merged 1 commit into from Nov 15, 2019
Merged

nautilus: msg/simple: reset in_seq_acked to zero when session is reset #29592

merged 1 commit into from Nov 15, 2019

Conversation

penglaiyxy
Copy link
Contributor

@penglaiyxy penglaiyxy commented Aug 12, 2019 •
edited

SimpleMessenger module has been removed from master branch in e57af1d, so I picked the nautilus branch to merge in.

Fixes: https://tracker.ceph.com/issues/41195
Signed-off-by: Xiangyang Yu penglaiyxy@gmail.com

@tchaikov tchaikov added the mgr label Aug 12, 2019
@penglaiyxy
Copy link
Contributor Author

@tchaikov The label is added "mgr"? I think it should be "msg".

@tchaikov tchaikov added messenger Issues involving one of the Ceph messenger implementations and removed mgr labels Aug 14, 2019
@tchaikov
Copy link
Contributor

thanks @penglaiyxy! fixed.

@tchaikov
Copy link
Contributor

tchaikov commented Aug 15, 2019 •
edited

@penglaiyxy the fix looks great! a couple things though:

  1. replace "[msg/simple]" with "msg/simple" in the title of the commit message
  2. add a section in the commit message explaining why this change is not cherry-picked from master -- i think it's because that master has removed SimpleMessenger in e57af1d
  3. target this change to mimic
  4. could you file a ticket on tracker so we can backport it to nautilus mimic
  5. and add a "Fixes:" line in the commit message to link the commit to that tracker ticket.

@penglaiyxy penglaiyxy changed the title [msg/simple] reset in_seq_acked to zero when session is reset nautilus:msg/simple:reset in_seq_acked to zero when session is reset Aug 15, 2019
@penglaiyxy
Copy link
Contributor Author

@tchaikov I have modified my commit title and added a section explaining why I picked the nautilus branch to merge in .

Ticket in tracker has been added too.

Can we backport it from nautilus to mimic ?

@tchaikov tchaikov changed the title nautilus:msg/simple:reset in_seq_acked to zero when session is reset nautilus: msg/simple: reset in_seq_acked to zero when session is reset Aug 15, 2019
@tchaikov
Copy link
Contributor

@penglaiyxy could you remove "nautilus:" from the title of your commit message?

Can we backport it from nautilus to mimic ?

sorry i was wrong. thought you were doing the other way around. =(

so all good =)

@tchaikov tchaikov added this to the nautilus milestone Aug 15, 2019
@smithfarm
Copy link
Contributor

@penglaiyxy As @tchaikov said - please change the commit title to:

msg/simple: reset in_seq_acked to zero when session is reset

(note the space after msg/simple: )

tchaikov
tchaikov approved these changes Aug 16, 2019
View reviewed changes
Copy link
Contributor

@tchaikov tchaikov left a comment

Choose a reason for hiding this comment

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

lgtm. i just don't like the title of the commit message.

@penglaiyxy
msg/simple: reset in_seq_acked to zero when session is reset
4b072a3 
The master has removed SimpleMessenger in e57af1d, so I picked
the nautilus branch to merge in.

Fixes: https://tracker.ceph.com/issues/41195
Signed-off-by: Xiangyang Yu <penglaiyxy@gmail.com>
@penglaiyxy
Copy link
Contributor Author

@smithfarm @tchaikov Commit message has been modified.

gregsfortytwo
gregsfortytwo approved these changes Aug 19, 2019
View reviewed changes
Copy link
Member

@gregsfortytwo gregsfortytwo left a comment

Choose a reason for hiding this comment

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

Reviewed-by: Greg Farnum gfarnum@redhat.com

@smithfarm smithfarm added the nautilus-batch-1 nautilus point releases label Oct 1, 2019
@yuriw
Copy link
Contributor

yuriw commented Nov 7, 2019

wip-yuri5-testing-2019-11-07-2203-nautilus

@yuriw
Copy link
Contributor

yuriw commented Nov 11, 2019

wip-yuri4-testing-2019-11-nautilus

@yuriw yuriw merged commit 0f2aea1 into ceph:nautilus Nov 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tchaikov tchaikov tchaikov approved these changes

@gregsfortytwo gregsfortytwo gregsfortytwo approved these changes

@rjfd rjfd Awaiting requested review from rjfd

Assignees
No one assigned
Labels
bug-fix messenger Issues involving one of the Ceph messenger implementations nautilus-batch-1 nautilus point releases needs-qa wip-yuri4-testing
Projects
None yet
Milestone
nautilus
6 participants
@penglaiyxy @tchaikov @smithfarm @yuriw @gregsfortytwo @liewegas