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

osd: delete one of the repeated op->mark_started in ReplicatedBackend::sub_op_modify_impl #9977

Merged
merged 1 commit into from Jul 25, 2016

Conversation

shun-s
Copy link
Contributor

@shun-s shun-s commented Jun 28, 2016

http://tracker.ceph.com/issues/16572

…o in ReplicatedBackend::sub_op_modify_impl

delete one mark_start event as there are two same op->mark_started in ReplicatedBackend::sub_op_modify_impl

Signed-off-by: shun-s song.shun3@zte.com.cn

@shun-s
Copy link
Contributor Author

shun-s commented Jun 28, 2016

when using ceph daemon osd.{id} dump_historic_op, it will show repeated info like bellow:

"description": "osd_repop(client.195731.0:41820 8.137  \ /b1cb0937\/rbd_data.1a77b238e1f29.0000000000001b66\/head v 3558'216067)",      

              "initiated_at": "2016-06-28 06:08:52.980918",    
              "age": 549.619780,   
              "duration": 9.557757,   
              "type_data": [  
               ....   
                    {  
                        "time": "2016-06-28 06:08:52.981251",
                        "event": "started"
                    },
                    {
                        "time": "2016-06-28 06:08:52.981375",
                        "event": "started"
                    }, 

@shun-s shun-s changed the title replcatedBackend: delete one useless op->mark_started as there are tw… replcatedBackend: delete one repeated op->mark_started as there are tw… Jun 30, 2016
@shun-s shun-s changed the title replcatedBackend: delete one repeated op->mark_started as there are tw… replcatedBackend: delete one of the repeated op->mark_started in ReplicatedBackend::sub_op_modify_impl Jun 30, 2016
@shun-s
Copy link
Contributor Author

shun-s commented Jun 30, 2016

@tchaikov would you please take a minute to review this pr, thanks

@tchaikov tchaikov added this to the hammer milestone Jul 1, 2016
@tchaikov
Copy link
Contributor

tchaikov commented Jul 1, 2016

@smithfarm i understand

we don't backport cleanup or features to LTS branches unless they are critical or requisite for backporting bug fixes. and in general, we cherry-pick commits from master.

but this change only exists in hammer, and removes the duplicated entries in the output of dump_historic_op.

personally, i am inclined to include this fix. what do you think?

@smithfarm
Copy link
Contributor

@tchaikov OK with me. I created http://tracker.ceph.com/issues/16572 to track. @dachary What do you think?

@smithfarm
Copy link
Contributor

@shun-s: Please amend the commit message so it includes a line:

Fixes: http://tracker.ceph.com/issues/16572

immediately prior to the "Signed-off-by" line. Thanks!

…o in ReplicatedBackend::sub_op_modify_impl

delete one mark_start event as there are two same op->mark_started  in ReplicatedBackend::sub_op_modify_impl
Fixes: http://tracker.ceph.com/issues/16572

Signed-off-by: shun-s <song.shun3@zte.com.cn>
@shun-s
Copy link
Contributor Author

shun-s commented Jul 3, 2016

@smithfarm sorry for seeing this patient review just now, and amend Fixes: http://tracker.ceph.com/issues/16572 is done, please take a look, thanks

@shun-s
Copy link
Contributor Author

shun-s commented Jul 13, 2016

@smithfarm please take a minute to review, much appreciate

@smithfarm
Copy link
Contributor

LGTM but will only be merged after it gets through integration testing, which is ongoing. Please be patient.

smithfarm added a commit that referenced this pull request Jul 18, 2016
…d op->mark_started in ReplicatedBackend::sub_op_modify_impl

Reviewed-by: Nathan Cutler <ncutler@suse.com>
smithfarm added a commit that referenced this pull request Jul 24, 2016
…d op->mark_started in ReplicatedBackend::sub_op_modify_impl

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

@tchaikov This PR is in the latest round of hammer-backports integration tests, which passed a rados run (the only failures are a valgrind false positive that has since been fixed by ceph/teuthology#915 and http://tracker.ceph.com/issues/15139 which is an infrastructure issue with two of the tests) - for details, see: http://tracker.ceph.com/issues/15895#note-18

OK to merge?

@tchaikov
Copy link
Contributor

@smithfarm yeah!

@smithfarm smithfarm merged commit 81133dd into ceph:hammer Jul 25, 2016
@smithfarm smithfarm changed the title replcatedBackend: delete one of the repeated op->mark_started in ReplicatedBackend::sub_op_modify_impl osd: delete one of the repeated op->mark_started in ReplicatedBackend::sub_op_modify_impl Aug 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants