You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2022/05/09 09:07:58 UTC
[GitHub] [iotdb] SpriCoder opened a new pull request, #5843: [IOTDB-3117][snapshot] add authInfo snapshot.
SpriCoder opened a new pull request, #5843:
URL: https://github.com/apache/iotdb/pull/5843
implement snapshot of authInfo.
When take snapshot, authInfo will copy both user folder and role folder into snapshot.
When load snapshot, authInfo will copy user folder and role folder from snapshot into system folder.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] MarcosZyk commented on a diff in pull request #5843: [IOTDB-3117][snapshot] add authInfo snapshot.
Posted by GitBox <gi...@apache.org>.
MarcosZyk commented on code in PR #5843:
URL: https://github.com/apache/iotdb/pull/5843#discussion_r867887666
##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/AuthorInfo.java:
##########
@@ -331,6 +348,121 @@ public PermissionInfoResp executeListUserPrivileges(AuthorReq plan) throws AuthE
}
}
+ @Override
+ public boolean processTakeSnapshot(File snapshotDir) throws TException, IOException {
+ SystemFileFactory systemFileFactory = SystemFileFactory.INSTANCE;
+ File userFolder = systemFileFactory.getFile(commonConfig.getUserFolder());
+ File userSnapshotDir = systemFileFactory.getFile(snapshotDir, userSnapshotFileName);
+ File userTmpSnapshotDir =
+ systemFileFactory.getFile(userSnapshotDir.getAbsolutePath() + "-" + UUID.randomUUID());
+ File roleFolder = systemFileFactory.getFile(commonConfig.getRoleFolder());
+ File roleSnapshotDir = systemFileFactory.getFile(snapshotDir, roleSnapshotFileName);
+ File roleTmpSnapshotDir =
+ systemFileFactory.getFile(roleSnapshotDir.getAbsolutePath() + "-" + UUID.randomUUID());
+
+ boolean result = true;
+ synchronized (IAuthorizer.class) {
+ try {
Review Comment:
It seems this synchronize is only used for avoid concurrent snapshot creation. The snapshot invoking program should take the responsibility to acquire the global write lock of configNode.
##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/AuthorInfo.java:
##########
@@ -331,6 +348,121 @@ public PermissionInfoResp executeListUserPrivileges(AuthorReq plan) throws AuthE
}
}
+ @Override
+ public boolean processTakeSnapshot(File snapshotDir) throws TException, IOException {
+ SystemFileFactory systemFileFactory = SystemFileFactory.INSTANCE;
+ File userFolder = systemFileFactory.getFile(commonConfig.getUserFolder());
+ File userSnapshotDir = systemFileFactory.getFile(snapshotDir, userSnapshotFileName);
+ File userTmpSnapshotDir =
+ systemFileFactory.getFile(userSnapshotDir.getAbsolutePath() + "-" + UUID.randomUUID());
+ File roleFolder = systemFileFactory.getFile(commonConfig.getRoleFolder());
+ File roleSnapshotDir = systemFileFactory.getFile(snapshotDir, roleSnapshotFileName);
+ File roleTmpSnapshotDir =
+ systemFileFactory.getFile(roleSnapshotDir.getAbsolutePath() + "-" + UUID.randomUUID());
+
+ boolean result = true;
+ synchronized (IAuthorizer.class) {
+ try {
Review Comment:
It seems this synchronize is only used for avoiding concurrent snapshot creation. The snapshot invoking program should take the responsibility to acquire the global write lock of configNode.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] qiaojialin merged pull request #5843: [IOTDB-3117][snapshot] add authInfo snapshot.
Posted by GitBox <gi...@apache.org>.
qiaojialin merged PR #5843:
URL: https://github.com/apache/iotdb/pull/5843
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] SpriCoder commented on pull request #5843: [IOTDB-3117][snapshot] add authInfo snapshot.
Posted by GitBox <gi...@apache.org>.
SpriCoder commented on PR #5843:
URL: https://github.com/apache/iotdb/pull/5843#issuecomment-1121832954
Addition: in this PR, I move FileUtils from server to node-commons.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] MarcosZyk commented on a diff in pull request #5843: [IOTDB-3117][snapshot] add authInfo snapshot.
Posted by GitBox <gi...@apache.org>.
MarcosZyk commented on code in PR #5843:
URL: https://github.com/apache/iotdb/pull/5843#discussion_r867885300
##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/AuthorInfo.java:
##########
@@ -331,6 +348,121 @@ public PermissionInfoResp executeListUserPrivileges(AuthorReq plan) throws AuthE
}
}
+ @Override
+ public boolean processTakeSnapshot(File snapshotDir) throws TException, IOException {
+ SystemFileFactory systemFileFactory = SystemFileFactory.INSTANCE;
+ File userFolder = systemFileFactory.getFile(commonConfig.getUserFolder());
+ File userSnapshotDir = systemFileFactory.getFile(snapshotDir, userSnapshotFileName);
+ File userTmpSnapshotDir =
+ systemFileFactory.getFile(userSnapshotDir.getAbsolutePath() + "-" + UUID.randomUUID());
+ File roleFolder = systemFileFactory.getFile(commonConfig.getRoleFolder());
+ File roleSnapshotDir = systemFileFactory.getFile(snapshotDir, roleSnapshotFileName);
+ File roleTmpSnapshotDir =
+ systemFileFactory.getFile(roleSnapshotDir.getAbsolutePath() + "-" + UUID.randomUUID());
+
+ boolean result = true;
+ synchronized (IAuthorizer.class) {
+ try {
Review Comment:
If some other authorInfo write operation doesn't synchronize its process, will this synchronize mechanism work?
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] qiaojialin commented on a diff in pull request #5843: [IOTDB-3117][snapshot] add authInfo snapshot.
Posted by GitBox <gi...@apache.org>.
qiaojialin commented on code in PR #5843:
URL: https://github.com/apache/iotdb/pull/5843#discussion_r872562785
##########
node-commons/src/main/java/org/apache/iotdb/commons/auth/user/LocalFileUserAccessor.java:
##########
@@ -55,10 +58,10 @@
* bytes
*/
public class LocalFileUserAccessor implements IUserAccessor {
-
+ private static final Logger logger = LoggerFactory.getLogger(LocalFileUserAccessor.class);
private static final String TEMP_SUFFIX = ".temp";
private static final String STRING_ENCODING = "utf-8";
- private static final Logger logger = LoggerFactory.getLogger(LocalFileUserAccessor.class);
+ private static final String userSnapshotFileName = "system" + File.separator + "users";
Review Comment:
get from CommonConfig?
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] SpriCoder commented on a diff in pull request #5843: [IOTDB-3117][snapshot] add authInfo snapshot.
Posted by GitBox <gi...@apache.org>.
SpriCoder commented on code in PR #5843:
URL: https://github.com/apache/iotdb/pull/5843#discussion_r868766319
##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/AuthorInfo.java:
##########
@@ -331,6 +348,121 @@ public PermissionInfoResp executeListUserPrivileges(AuthorReq plan) throws AuthE
}
}
+ @Override
+ public boolean processTakeSnapshot(File snapshotDir) throws TException, IOException {
+ SystemFileFactory systemFileFactory = SystemFileFactory.INSTANCE;
+ File userFolder = systemFileFactory.getFile(commonConfig.getUserFolder());
+ File userSnapshotDir = systemFileFactory.getFile(snapshotDir, userSnapshotFileName);
+ File userTmpSnapshotDir =
+ systemFileFactory.getFile(userSnapshotDir.getAbsolutePath() + "-" + UUID.randomUUID());
+ File roleFolder = systemFileFactory.getFile(commonConfig.getRoleFolder());
+ File roleSnapshotDir = systemFileFactory.getFile(snapshotDir, roleSnapshotFileName);
+ File roleTmpSnapshotDir =
+ systemFileFactory.getFile(roleSnapshotDir.getAbsolutePath() + "-" + UUID.randomUUID());
+
+ boolean result = true;
+ synchronized (IAuthorizer.class) {
+ try {
Review Comment:
I use ReentrantReadWriteLock in AuthorizerManager to fix this problem.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org