sijie commented on a change in pull request #301: Issue-299
EnsemblePlacementPolicy in 4.5 is not compatible with 4.4 clients
URL: https://github.com/apache/bookkeeper/pull/301#discussion_r129767278
##########
File path:
bookkeeper-server/src/test/java/org/apache/bookkeeper/client/GenericEnsemblePlacementPolicyTest.java
##########
@@ -132,9 +132,9 @@ public void testReplaceBookie() throws Exception {
lh.addEntry(value);
}
}
- assertEquals(2, customMetadataOnNewEnsembleStack.size());
Review comment:
I see. the callers are the new bookkeeper client. if that's the case, I am
not sure if we should support such backward compatibility. to me, it is not a
simple backward compatibility issue, it is a semantic issue.
for example, the new placement policy introduces ackQuorumSize in
newEnsemble and replica settings for replaceBookie. The new bookkeeper client
expects the placement policy to be respecting the replica settings when
choosing bookies. However the old placement policy implementation doesn't
really respect these settings because it was implemented lack of those
settings. This is a semantic disconnection between new client behavior and old
placement policy implementation. I am not sure if this is a behavior that we
should expose to applications, because there might be a lot of unknowns when
new client uses old placement policy. same concerns applied even if we want to
refactor it to use structures.
but anyway, I am fine the approach in this pull request, but we might
consider marking the old methods as deprecated. so that people can upgrade to
use new interface in future.
We might need to discuss the audience and stability of interfaces that
exposed by bookkeeper in next release.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
