ctubbsii commented on code in PR #5192:
URL: https://github.com/apache/accumulo/pull/5192#discussion_r1901352131
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java:
##########
@@ -678,7 +678,7 @@ public long getFlushID() throws NoNodeException {
try {
String zTablePath = tabletServer.getContext().getZooKeeperRoot() +
Constants.ZTABLES + "/"
+ extent.tableId() + Constants.ZTABLE_FLUSH_ID;
- String id = new String(context.getZooReaderWriter().getData(zTablePath),
UTF_8);
+ String id = new
String(context.getZooSession().asReaderWriter().getData(zTablePath), UTF_8);
Review Comment:
There is a difference between these methods, though. ZR/ZRW's purpose was to
simplify ZK interactions by providing similar methods that 1) reduce the need
to provide useless required parameters that were always passed with a specific
value (often `null` or `false` for a watcher or `-1` for a version), and 2) to
provide an automatic retry mechanism on transient failures.
So, the code that is using ZR/ZRW should be assumed to be written with the
retry behavior in mind (either because the developer explicitly wanted retry or
doesn't care), and the code calling the similar methods on ZooSession directly
(previously it would have called ZooKeeper directly) should be assumed to be
written with no desire to retry (often because it has its own retry behavior)
or because it needs to pass specific values. Changing from one to the other
should be done very carefully, and should be done in a way that reviews the
calling code that is being changed to examine the implications. I didn't change
any of those, because that's too much work for this PR. However, that was one
of my suggestions for follow-on work, to perhaps collapse ZR/ZRW with
ZooSession to provide retry-on-transient failures to all methods automatically
on ZooSession. And also possibly to provide the overloaded method versions from
ZR/ZRW to ZooSession that simplify the need to pass parameters with
common values. Doing those could possibly lead to the removal of ZR/ZRW, but
might diverge ZooSession from being a ZooKeeper API look-alike to a more
general utility around ZooKeeper for all our needs. I'm not sure how far we
want to take that, but it can be done as a follow-on either way, with care
taken to examine the needs of the calling code in each case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
