Re: [PR] RATIS-2281. Make LogSegmentStartEnd getStartIndex and getEndIndex public [ratis]
devabhishekpal closed pull request #1321: RATIS-2281. Make LogSegmentStartEnd getStartIndex and getEndIndex public URL: https://github.com/apache/ratis/pull/1321 -- 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]
Re: [PR] RATIS-2281. Make LogSegmentStartEnd getStartIndex and getEndIndex public [ratis]
devabhishekpal commented on PR #1321: URL: https://github.com/apache/ratis/pull/1321#issuecomment-3676780242 Thanks for the inputs @errose28 @szetszwo . Closing this PR. -- 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]
Re: [PR] RATIS-2281. Make LogSegmentStartEnd getStartIndex and getEndIndex public [ratis]
szetszwo commented on PR #1321: URL: https://github.com/apache/ratis/pull/1321#issuecomment-3676089201 Sure, let's close this. Thanks for your understanding. -- 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]
Re: [PR] RATIS-2281. Make LogSegmentStartEnd getStartIndex and getEndIndex public [ratis]
errose28 commented on PR #1321: URL: https://github.com/apache/ratis/pull/1321#issuecomment-3676053137 > 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? -- 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]
Re: [PR] RATIS-2281. Make LogSegmentStartEnd getStartIndex and getEndIndex public [ratis]
szetszwo commented on PR #1321: URL: https://github.com/apache/ratis/pull/1321#issuecomment-3666418480 > The class is already public so it is part of the API. ... For servers, only the class 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. > ... 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. Note that if the tool is in Ratis, it can access the private API in the its implementation. It is easier to define a public tool API than a public server API. In this case, if LogSegmentStartEnd is a public API, it means that we have to support such segment files forever. Then, it will limit the flexibility of the underlying implementation (e.g. RATIS-2370 SegmentedRaftLog v2). -- 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]
Re: [PR] RATIS-2281. Make LogSegmentStartEnd getStartIndex and getEndIndex public [ratis]
errose28 commented on PR #1321: URL: https://github.com/apache/ratis/pull/1321#issuecomment-3662859003 > 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. -- 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]
Re: [PR] RATIS-2281. Make LogSegmentStartEnd getStartIndex and getEndIndex public [ratis]
devabhishekpal commented on PR #1321: URL: https://github.com/apache/ratis/pull/1321#issuecomment-3633776309 @szetszwo by ratis log tool you are referring to the OM Ratis log repair tool? -- 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]
Re: [PR] RATIS-2281. Make LogSegmentStartEnd getStartIndex and getEndIndex public [ratis]
devabhishekpal commented on PR #1321: URL: https://github.com/apache/ratis/pull/1321#issuecomment-3633719384 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? -- 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]
Re: [PR] RATIS-2281. Make LogSegmentStartEnd getStartIndex and getEndIndex public [ratis]
szetszwo commented on PR #1321: URL: https://github.com/apache/ratis/pull/1321#issuecomment-3633669783 @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 ) -- 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]
Re: [PR] RATIS-2281. Make LogSegmentStartEnd getStartIndex and getEndIndex public [ratis]
devabhishekpal commented on PR #1321: URL: https://github.com/apache/ratis/pull/1321#issuecomment-355023 @szetszwo could you take a look as well? -- 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]
Re: [PR] RATIS-2281. Make LogSegmentStartEnd getStartIndex and getEndIndex public [ratis]
szetszwo commented on PR #1321: URL: https://github.com/apache/ratis/pull/1321#issuecomment-3577379143 @devabhishekpal , let's make sure how the methods are going to be used before changing them. -- 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]
Re: [PR] RATIS-2281. Make LogSegmentStartEnd getStartIndex and getEndIndex public [ratis]
devabhishekpal commented on PR #1321:
URL: https://github.com/apache/ratis/pull/1321#issuecomment-3566627512
@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()
}
}
```
--
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]
