apache
RATIS-2281. Make LogSegmentStartEnd getStartIndex and getEndIndex public
What changes were proposed in this pull request?
We are changing the visibility of LogSegmentStartEnd#getStartIndex() and LogSegmentStartEnd#getEndIndex() to public.
This will allow external tools that deal with the RATIS log, to directly make changes to it.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/RATIS-2281
How was this patch tested?
Patch was tested using unit tests
@errose28 I am not too sure of the use case, do you think this change suffices?
Or did the ticket mean to add new methods to LogSegmentPath class which expose the getStartIndex and getEndIndex.
as in:
class LogSegmentPath {
....
public long getStartIndex() {
return this.startEnd.getStartIndex()
}
public long getEndIndex() {
return this.startEnd.getEndIndex()
}
}
@devabhishekpal , let's make sure how the methods are going to be used before changing them.
@devabhishekpal , @errose28 ,
I think about this change. It seems not a good idea to simple changing the methods to public and let other projects using it. We should have some kind of APIs.
I wonder if the we could move the RaftLog tool from Ozone to Ratis?
As a workaround, Ozone could add a RaftLogUtils class in the org.apache.ratis.server.raftlog.segmented package for accessing the package private methods in the meantime. (Something similar to https://github.com/apache/ozone/blob/master/hadoop-hdds/common/src/main/java/com/google/protobuf/ProtoUtils.java )
Hmm, seems like a good long term plan. Maybe other projects dependent on RATIS can also use such tools. @errose28 what do you think of the effort on this shift of RaftLog tools from Ozone to Ratis?
@szetszwo by ratis log tool you are referring to the OM Ratis log repair tool?
I think about this change. It seems not a good idea to simple changing the methods to public and let other projects using it. We should have some kind of APIs.
The class is already public so it is part of the API. We are not changing arbitrary methods to public either. The class is named LogSegmentStartEnd, so it seems reasonable for it to expose read-only methods to retrieve the start and end indices that it is named for. The compareTo method which works on these indices is public as well.
I wonder if the we could move the RaftLog tool from Ozone to Ratis?
This might be possible but the implementation may look strange. The tool is to replace the message contents of one raft log entry with a different type of message/content which is specific to Ozone. An implementation in Ratis that takes a byte string to replace the log entry with could be used for this. However I don't see how adding a low level mutator to the public API is less harmful than two read-only methods in an already public class.
As a workaround, Ozone could add a RaftLogUtils class in the org.apache.ratis.server.raftlog.segmented package for accessing the package private methods in the meantime.
I'd rather not do a hack like this if we have the above two options available.
The class is already public so it is part of the API. ...
For servers, only the classes in ratis-server-api are public APIs. Public classes in ratis-server are not.
... The tool is to replace the message contents of one raft log entry with a different type of message/content which is specific to Ozone. An implementation in Ratis that takes a byte string to replace the log entry with could be used for this.
If the tool is in Ratis, it could let the applications such as Ozone pass a function for doing the replacement.
Note that if the tool is in Ratis, it can access the private API in its implementation.
... I don't see how adding a low level mutator to the public API is less harmful than two read-only methods in an already public class.
It is easier to define a public tool API than a public server API. In this case, if LogSegmentStartEnd is a public server API, it means that we have to support such segment files forever. Then, it limits the flexibility of the underlying implementation (e.g. RATIS-2370 SegmentedRaftLog v2).
If the tool is in Ratis, it could let the applications such as Ozone pass a function for doing the replacement.
I agree this would probably be the best thing long term. We can file a Jira for this and migrate Ozone when it is ready. In the mean time I think we can keep using the regex workaround to identify log segments rather than the private package approach. Should we close this PR?