Bump Khepri to 0.17.0 #12753
Bump Khepri to 0.17.0 #12753
Conversation
the-mikedavis
force-pushed
the
md/khepri-0-17
branch
3 times, most recently
from
6d289fe to
cc6185d
Compare
the-mikedavis
force-pushed
the
md/khepri-0-17
branch
4 times, most recently
from
7d8682d to
4034f96
Compare
Khepri v0.17 needs a change in Ra 2.15.0 that exposes more data in the `ra_aux` API.
the-mikedavis
force-pushed
the
md/khepri-0-17
branch
2 times, most recently
from
5cb0a79 to
42f571a
Compare
| case khepri_tx_adv:delete(Path) of | ||
| %% Khepri 0.16 and below returned `khepri:node_props()' for | ||
| %% adv queries and commands targeting one node: | ||
| {ok, #{data := _}} -> | ||
| %% we want to execute some things, as decided by rabbit_exchange, | ||
| %% after the transaction. | ||
| rabbit_db_binding:delete_for_destination_in_khepri(QueueName, OnlyDurable); | ||
| %% Khepri 0.17+ returns `khepri_adv:node_props_map()` | ||
| %% instead. | ||
| {ok, #{Path := #{data := _}}} -> | ||
| rabbit_db_binding:delete_for_destination_in_khepri(QueueName, OnlyDurable); |
There was a problem hiding this comment.
Oh, that’s a good point! This probably needs a fix in Khepri. I see two solutions:
- The typespec is updated to document the old return value (which it can return)
- The function converts the old return value to the new one (I didn’t check if it was possible though)
What do you think?
There was a problem hiding this comment.
Do we need a change in Khepri? rabbitmq/khepri#303 only touched the "client" code (i.e. not khepri_machine) so older versions should continue to see khepri:node_props() while newer versions consistently see khepri_adv:node_props_map(), even if the leader were an older versioned node.
The typespec change is unfortunate and the dialyzer had a few complaints that I worked around with -dialyzer({nowarn_function, [..]}). attributes. I think this is ok for compatibility code like this. We should be able to delete these branches and remove the dialyzer ignores for 4.2.x once we don't need compatibility with Khepri 0.16. (Assuming Khepri 0.17 would ship with 4.1.x.)
There was a problem hiding this comment.
newer versions consistently see khepri_adv:node_props_map(), even if the leader were an older versioned node.
Right, we have reply_from => LocalMember for commands, so the return value is created by the local version of Khepri. Then, why do we need to have the case for both return values?
With the discussions around long-term support for RabbitMQ, compatibility code can stay for a long time as we will have to support upgrades that skip versions. That said, it might not be the case with RabbitMQ 4.0.x. This is more of a general comment.
There was a problem hiding this comment.
Oh actually that's a really good point, I suppose the old return value isn't possible and we shouldn't need this compatibility code
the-mikedavis
force-pushed
the
md/khepri-0-17
branch
from
42f571a to
2d1225d
Compare
`locally_known_members/1` and `locally_known_node/1` were replaced with `members/2` and `nodes/2` with `favor` set to `low_latency` - this matches the interface for queries in Khepri.
Khepri 0.17.x will change the return of functions from the khepri_adv and khepri_tx_adv modules. Previously, functions that target one specific tree node, for example `khepri_adv:delete/3`, would return the node props map (`khepri:node_props()`) for the affected node. Now all of the "adv API" returns `khepri_adv:node_props_map()` for consistency.
the-mikedavis
force-pushed
the
md/khepri-0-17
branch
from
2d1225d to
17e7652
Compare
This is not ready to be merged yet, Khepri 0.17.0 hasn't yet been released. (And we want to make more changes before then anyways.)
This PR handles the breaking changes in Khepri 0.17.0.