msg/async: Encode message once features are set#49619
Conversation
739703a to
36e5554
Compare
|
@rzarzynski I think you have already gone through the fix for ProtocolV2, could you take a look at the changes I made for ProtocolV1? |
|
This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days. |
|
This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days. |
|
This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days. |
|
@rzarzynski @amathuria Is this still relevant? |
@athanatos sorry I missed this. This is still relevant. The ProtocolV1 fix is pending review. |
|
@amathuria: ping. |
36e5554 to
c3ca13a
Compare
|
Testing the changes here: https://pulpito.ceph.com/amathuri-2024-03-25_13:28:33-rados:verify-main-distro-default-smithi/ |
rzarzynski
left a comment
There was a problem hiding this comment.
I'm afraid there is a logic error. Please see my comments.
Modify send_message to check if features are set before trying to encode a message. If features are not set at this point, we will encode the message at a later stage (in write_event) when the connection will be in ready state which implies that the features will definitely be set. Fixes: https://tracker.ceph.com/issues/52657 Signed-off-by: Aishwarya Mathuria <amathuri@redhat.com>
c3ca13a to
7268211
Compare
| Message *ProtocolV1::_get_next_outgoing(ceph::buffer::list *bl) { | ||
| Message *m = 0; | ||
| ProtocolV1::out_q_entry_t ProtocolV1::_get_next_outgoing() { | ||
| out_q_entry_t out_entry; |
There was a problem hiding this comment.
OK, it's defaulted-constructed with nullptr, false and empty bl.
|
jenkins retest this please |
|
This PR is under test in https://tracker.ceph.com/issues/65560. |
Modify send_message to check if features are set before trying to encode a message.
If features are not set at this point, we will encode the message at a later stage (in write_event) when the connection will be in ready state which implies that the features will definitely be set.
Fixes: https://tracker.ceph.com/issues/52657
Signed-off-by: Aishwarya Mathuria amathuri@redhat.com
Contribution Guidelines
To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "pacific"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
Checklist
Show available Jenkins commands
jenkins retest this pleasejenkins test classic perfjenkins test crimson perfjenkins test signedjenkins test make checkjenkins test make check arm64jenkins test submodulesjenkins test dashboardjenkins test dashboard cephadmjenkins test apijenkins test docsjenkins render docsjenkins test ceph-volume alljenkins test ceph-volume toxjenkins test windows