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 2021/09/27 07:36:26 UTC
[GitHub] [iotdb] jamber001 opened a new pull request #4042: [IOTDB-1749] sync-tool's lockInstance() dose not take effect
jamber001 opened a new pull request #4042:
URL: https://github.com/apache/iotdb/pull/4042
[IOTDB-1749] sync-tool's lockInstance() dose not take effect
--
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] coveralls edited a comment on pull request #4042: [IOTDB-1749] sync-tool's lockInstance() dose not take effect
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #4042:
URL: https://github.com/apache/iotdb/pull/4042#issuecomment-927641352
[](https://coveralls.io/builds/43107992)
Coverage increased (+0.02%) to 67.481% when pulling **7b09e617171f238562966c3ae032b7258c80a214 on jamber001:IOTDB-1749** into **1e12e6581b03b40956184a610063c2198466afbd on apache:master**.
--
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] coveralls edited a comment on pull request #4042: [IOTDB-1749] sync-tool's lockInstance() dose not take effect
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #4042:
URL: https://github.com/apache/iotdb/pull/4042#issuecomment-927641352
--
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] wangchao316 commented on a change in pull request #4042: [IOTDB-1749] sync-tool's lockInstance() dose not take effect
Posted by GitBox <gi...@apache.org>.
wangchao316 commented on a change in pull request #4042:
URL: https://github.com/apache/iotdb/pull/4042#discussion_r716477308
##########
File path: server/src/main/java/org/apache/iotdb/db/sync/sender/transfer/SyncClient.java
##########
@@ -171,7 +171,8 @@ public void verifySingleton() throws IOException {
* @param lockFile lock file
*/
private boolean lockInstance(File lockFile) {
- try (final RandomAccessFile randomAccessFile = new RandomAccessFile(lockFile, "rw")) {
+ try {
+ final RandomAccessFile randomAccessFile = new RandomAccessFile(lockFile, "rw");
Review comment:
randomAccessFile.close() will be not call?
--
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] jamber001 commented on a change in pull request #4042: [IOTDB-1749] sync-tool's lockInstance() dose not take effect
Posted by GitBox <gi...@apache.org>.
jamber001 commented on a change in pull request #4042:
URL: https://github.com/apache/iotdb/pull/4042#discussion_r716509270
##########
File path: server/src/main/java/org/apache/iotdb/db/sync/sender/transfer/SyncClient.java
##########
@@ -171,7 +171,8 @@ public void verifySingleton() throws IOException {
* @param lockFile lock file
*/
private boolean lockInstance(File lockFile) {
- try (final RandomAccessFile randomAccessFile = new RandomAccessFile(lockFile, "rw")) {
+ try {
+ final RandomAccessFile randomAccessFile = new RandomAccessFile(lockFile, "rw");
Review comment:
> randomAccessFile.close() will be not call?
Please review next 15 line of codes below this change.
randomAccessFile will be released when Process exit.
--
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] coveralls commented on pull request #4042: [IOTDB-1749] sync-tool's lockInstance() dose not take effect
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #4042:
URL: https://github.com/apache/iotdb/pull/4042#issuecomment-927641352
[](https://coveralls.io/builds/43080842)
Coverage decreased (-0.04%) to 67.486% when pulling **e9f48af84f489e30784bf8480e5f34e52fde503f on jamber001:IOTDB-1749** into **87e1ae444523c07b66b7bde1ec63d6b1924e3fca on apache:master**.
--
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] coveralls edited a comment on pull request #4042: [IOTDB-1749] sync-tool's lockInstance() dose not take effect
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #4042:
URL: https://github.com/apache/iotdb/pull/4042#issuecomment-927641352
[](https://coveralls.io/builds/43107989)
Coverage increased (+0.01%) to 67.469% when pulling **7b09e617171f238562966c3ae032b7258c80a214 on jamber001:IOTDB-1749** into **1e12e6581b03b40956184a610063c2198466afbd on apache:master**.
--
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] jamber001 commented on a change in pull request #4042: [IOTDB-1749] sync-tool's lockInstance() dose not take effect
Posted by GitBox <gi...@apache.org>.
jamber001 commented on a change in pull request #4042:
URL: https://github.com/apache/iotdb/pull/4042#discussion_r716510212
##########
File path: server/src/main/java/org/apache/iotdb/db/sync/sender/transfer/SyncClient.java
##########
@@ -171,7 +171,8 @@ public void verifySingleton() throws IOException {
* @param lockFile lock file
*/
private boolean lockInstance(File lockFile) {
- try (final RandomAccessFile randomAccessFile = new RandomAccessFile(lockFile, "rw")) {
+ try {
+ final RandomAccessFile randomAccessFile = new RandomAccessFile(lockFile, "rw");
Review comment:
--
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] HTHou commented on pull request #4042: [IOTDB-1749] sync-tool's lockInstance() dose not take effect
Posted by GitBox <gi...@apache.org>.
HTHou commented on pull request #4042:
URL: https://github.com/apache/iotdb/pull/4042#issuecomment-927612675
@yschengzi
--
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] jamber001 closed pull request #4042: [IOTDB-1749] sync-tool's lockInstance() dose not take effect
Posted by GitBox <gi...@apache.org>.
jamber001 closed pull request #4042:
URL: https://github.com/apache/iotdb/pull/4042
--
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] jamber001 commented on a change in pull request #4042: [IOTDB-1749] sync-tool's lockInstance() dose not take effect
Posted by GitBox <gi...@apache.org>.
jamber001 commented on a change in pull request #4042:
URL: https://github.com/apache/iotdb/pull/4042#discussion_r717161309
##########
File path: server/src/main/java/org/apache/iotdb/db/sync/sender/transfer/SyncClient.java
##########
@@ -171,7 +171,8 @@ public void verifySingleton() throws IOException {
* @param lockFile lock file
*/
private boolean lockInstance(File lockFile) {
- try (final RandomAccessFile randomAccessFile = new RandomAccessFile(lockFile, "rw")) {
+ try {
+ final RandomAccessFile randomAccessFile = new RandomAccessFile(lockFile, "rw");
Review comment:
@wangchao316 Has my reply answer your question?
##########
File path: server/src/main/java/org/apache/iotdb/db/sync/sender/transfer/SyncClient.java
##########
@@ -171,7 +171,8 @@ public void verifySingleton() throws IOException {
* @param lockFile lock file
*/
private boolean lockInstance(File lockFile) {
- try (final RandomAccessFile randomAccessFile = new RandomAccessFile(lockFile, "rw")) {
+ try {
+ final RandomAccessFile randomAccessFile = new RandomAccessFile(lockFile, "rw");
Review comment:
> The first thread acquires the lock, addShutdownHook.
> The second thread executes new RandomAccessFile, but tryLock() fails. Do you want to close RandomAccessFile?
> We can't just rely on close in the hook. If there is no hook, there is no close file.
> @jamber001
I understand your thought, now.
Though this will not affect sync-tool process, but from function view, it is better to close RandomAccessFile.
I will add codes for this.
--
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] wangchao316 commented on a change in pull request #4042: [IOTDB-1749] sync-tool's lockInstance() dose not take effect
Posted by GitBox <gi...@apache.org>.
wangchao316 commented on a change in pull request #4042:
URL: https://github.com/apache/iotdb/pull/4042#discussion_r717167610
##########
File path: server/src/main/java/org/apache/iotdb/db/sync/sender/transfer/SyncClient.java
##########
@@ -171,7 +171,8 @@ public void verifySingleton() throws IOException {
* @param lockFile lock file
*/
private boolean lockInstance(File lockFile) {
- try (final RandomAccessFile randomAccessFile = new RandomAccessFile(lockFile, "rw")) {
+ try {
+ final RandomAccessFile randomAccessFile = new RandomAccessFile(lockFile, "rw");
Review comment:
The first thread acquires the lock, addShutdownHook.
The second thread executes new RandomAccessFile, but tryLock() fails. Do you want to close RandomAccessFile?
We can't just rely on close in the hook. If there is no hook, there is no close file.
@jamber001
--
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] wangchao316 commented on a change in pull request #4042: [IOTDB-1749] sync-tool's lockInstance() dose not take effect
Posted by GitBox <gi...@apache.org>.
wangchao316 commented on a change in pull request #4042:
URL: https://github.com/apache/iotdb/pull/4042#discussion_r717167610
##########
File path: server/src/main/java/org/apache/iotdb/db/sync/sender/transfer/SyncClient.java
##########
@@ -171,7 +171,8 @@ public void verifySingleton() throws IOException {
* @param lockFile lock file
*/
private boolean lockInstance(File lockFile) {
- try (final RandomAccessFile randomAccessFile = new RandomAccessFile(lockFile, "rw")) {
+ try {
+ final RandomAccessFile randomAccessFile = new RandomAccessFile(lockFile, "rw");
Review comment:
The first thread acquires the lock, addShutdownHook.
The second thread executes new RandomAccessFile, but tryLock() fails. Do you want to close RandomAccessFile?
We can't just rely on close in the hook. If there is no hook, there is no close file.
@jamber001
--
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] jamber001 commented on a change in pull request #4042: [IOTDB-1749] sync-tool's lockInstance() dose not take effect
Posted by GitBox <gi...@apache.org>.
jamber001 commented on a change in pull request #4042:
URL: https://github.com/apache/iotdb/pull/4042#discussion_r717173382
##########
File path: server/src/main/java/org/apache/iotdb/db/sync/sender/transfer/SyncClient.java
##########
@@ -171,7 +171,8 @@ public void verifySingleton() throws IOException {
* @param lockFile lock file
*/
private boolean lockInstance(File lockFile) {
- try (final RandomAccessFile randomAccessFile = new RandomAccessFile(lockFile, "rw")) {
+ try {
+ final RandomAccessFile randomAccessFile = new RandomAccessFile(lockFile, "rw");
Review comment:
> The first thread acquires the lock, addShutdownHook.
> The second thread executes new RandomAccessFile, but tryLock() fails. Do you want to close RandomAccessFile?
> We can't just rely on close in the hook. If there is no hook, there is no close file.
> @jamber001
I understand your thought, now.
Though this will not affect sync-tool process, but from function view, it is better to close RandomAccessFile.
I will add codes for this.
--
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] jamber001 commented on a change in pull request #4042: [IOTDB-1749] sync-tool's lockInstance() dose not take effect
Posted by GitBox <gi...@apache.org>.
jamber001 commented on a change in pull request #4042:
URL: https://github.com/apache/iotdb/pull/4042#discussion_r717161309
##########
File path: server/src/main/java/org/apache/iotdb/db/sync/sender/transfer/SyncClient.java
##########
@@ -171,7 +171,8 @@ public void verifySingleton() throws IOException {
* @param lockFile lock file
*/
private boolean lockInstance(File lockFile) {
- try (final RandomAccessFile randomAccessFile = new RandomAccessFile(lockFile, "rw")) {
+ try {
+ final RandomAccessFile randomAccessFile = new RandomAccessFile(lockFile, "rw");
Review comment:
@wangchao316 Has my reply answer your question?
--
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] jamber001 closed pull request #4042: [IOTDB-1749] sync-tool's lockInstance() dose not take effect
Posted by GitBox <gi...@apache.org>.
jamber001 closed pull request #4042:
URL: https://github.com/apache/iotdb/pull/4042
--
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