ivanzlenko commented on code in PR #7969:
URL: https://github.com/apache/ignite-3/pull/7969#discussion_r3077756649
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/storage/segstore/SegmentLogStorageManager.java:
##########
@@ -63,17 +72,13 @@ public SegmentLogStorageManager(
}
@Override
- public LogStorage createLogStorage(String groupId, RaftOptions
raftOptions) {
- return new SegstoreLogStorage(convertGroupId(groupId), fileManager);
+ public LogStorage createLogStorage(String raftNodeStorageId, RaftOptions
raftOptions) {
+ return new SegstoreLogStorage(convertNodeId(raftNodeStorageId),
fileManager);
}
@Override
- public void destroyLogStorage(String groupId) {
- try {
- fileManager.reset(convertGroupId(groupId), 1);
- } catch (IOException e) {
- throw new LogStorageException("Failed to destroy log storage for
group " + groupId, e);
- }
+ public void destroyLogStorage(String raftNodeStorageId) {
+ // TODO IGNITE-28527 Implement.
Review Comment:
Is it okay to brake actual production code?
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/storage/segstore/SegmentLogStorageManager.java:
##########
@@ -108,21 +113,46 @@ public CompletableFuture<Void> stopAsync(ComponentContext
componentContext) {
}
}
- private static long convertGroupId(String groupId) {
- if ("metastorage_group".equals(groupId)) {
- return 1;
+ /**
+ * Converts a raft node storage ID string to a unique positive long, used
to identify a logical log within the segment file manager.
+ *
+ * <p>For partition groups ("{objectId}_part_{partitionId}-{peerIdx}"):
+ * result = (objectId << 32 | partitionId) + {@link
#SPECIAL_GROUP_ID_OFFSET}.
+ * objectId and partitionId are non-negative ints, so the result is always
positive and unique per (objectId, partitionId) pair.
+ *
+ * <p>peerIdx is not encoded because a single Ignite node participates as
exactly one peer per raft group,
+ * so (objectId, partitionId) is already unique within one {@link
SegmentLogStorageManager}.
+ */
+ // TODO IGNITE-26977 Revise after changing partition ID from int to long.
+ public static long convertNodeId(String nodeId) {
Review Comment:
Are you sure it needs to be public? If it is just for test purposes I
would've rather test it through existing public API.
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/storage/segstore/SegmentLogStorageManager.java:
##########
@@ -108,21 +113,46 @@ public CompletableFuture<Void> stopAsync(ComponentContext
componentContext) {
}
}
- private static long convertGroupId(String groupId) {
- if ("metastorage_group".equals(groupId)) {
- return 1;
+ /**
+ * Converts a raft node storage ID string to a unique positive long, used
to identify a logical log within the segment file manager.
+ *
+ * <p>For partition groups ("{objectId}_part_{partitionId}-{peerIdx}"):
+ * result = (objectId << 32 | partitionId) + {@link
#SPECIAL_GROUP_ID_OFFSET}.
Review Comment:
Worth wrapping into code
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/storage/LogStorageManager.java:
##########
@@ -32,16 +32,16 @@ public interface LogStorageManager extends IgniteComponent {
/**
* Creates a log storage.
*
- * @param groupId Group ID to create storage for.
Review Comment:
Still would argue that rename should be a part of different PR
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/storage/segstore/SegmentLogStorageManager.java:
##########
@@ -108,21 +113,46 @@ public CompletableFuture<Void> stopAsync(ComponentContext
componentContext) {
}
}
- private static long convertGroupId(String groupId) {
- if ("metastorage_group".equals(groupId)) {
- return 1;
+ /**
+ * Converts a raft node storage ID string to a unique positive long, used
to identify a logical log within the segment file manager.
+ *
+ * <p>For partition groups ("{objectId}_part_{partitionId}-{peerIdx}"):
+ * result = (objectId << 32 | partitionId) + {@link
#SPECIAL_GROUP_ID_OFFSET}.
+ * objectId and partitionId are non-negative ints, so the result is always
positive and unique per (objectId, partitionId) pair.
+ *
+ * <p>peerIdx is not encoded because a single Ignite node participates as
exactly one peer per raft group,
+ * so (objectId, partitionId) is already unique within one {@link
SegmentLogStorageManager}.
+ */
+ // TODO IGNITE-26977 Revise after changing partition ID from int to long.
+ public static long convertNodeId(String nodeId) {
Review Comment:
It is better to expose package-private in SegmentLogStorage than make this
method public.
--
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]
