rpuch commented on code in PR #5083:
URL: https://github.com/apache/ignite-3/pull/5083#discussion_r1933644961
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/server/KeyValueStorage.java:
##########
@@ -237,15 +237,16 @@ public interface KeyValueStorage extends
ManuallyCloseable {
* Saves an arbitrary storage configuration as a byte array.
*
* @param configuration Configuration bytes.
- * @param index Operation's index.
- * @param term Operation's term.
+ * @param lastAppliedIndex Last applied index.
+ * @param lastAppliedTerm Last applied term.
*/
- void saveConfiguration(byte[] configuration, long index, long term);
+ void saveConfigurationWithLastAppliedIndexAndTerm(byte[] configuration,
long lastAppliedIndex, long lastAppliedTerm);
Review Comment:
It seems that it's enough to rename parameters. `saveConfiguration()` is ok
as a name; the caller will not be confused about what kind of index+term they
need to pass
##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/StateMachine.java:
##########
@@ -102,8 +102,10 @@ public interface StateMachine {
* {@link #onConfigurationCommitted(Configuration)} as full configuration
entry is provided.
*
* @param conf committed configuration
+ * @param lastAppliedIndex Last applied index.
+ * @param lastAppliedTerm Last applied term.
*/
- default void onRawConfigurationCommitted(ConfigurationEntry conf) {
+ default void
onConfigurationCommittedWithLastAppliedIndexAndTerm(ConfigurationEntry conf,
long lastAppliedIndex, long lastAppliedTerm) {
Review Comment:
Same thing about renaming: it seems that the old name is ok as the
parameters already explain what they are for
##########
modules/raft-api/src/main/java/org/apache/ignite/internal/raft/service/RaftGroupListener.java:
##########
@@ -61,8 +61,14 @@ class ShutdownException extends RuntimeException {
* Called when a configuration is committed (that is, written to a
majority of the group).
*
* @param config Configuration that was committed.
+ * @param lastAppliedIndex Last applied index.
+ * @param lastAppliedTerm Last applied term.
*/
- default void onConfigurationCommitted(CommittedConfiguration config) {
+ default void onConfigurationCommittedWithLastAppliedIndexAndTerm(
Review Comment:
Same thing: I don't think we should rename the method as the parameters
already make it clear what index is saved
--
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]
