You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@zookeeper.apache.org by GitBox <gi...@apache.org> on 2021/10/25 00:38:41 UTC

[GitHub] [zookeeper] Shoothzj opened a new pull request #1766: ZOOKEEPER-4387 Close ZkDb when ZooKeeper Server shutdown

Shoothzj opened a new pull request #1766:
URL: https://github.com/apache/zookeeper/pull/1766


   Close ZkDb when ZooKeeper Server shutdown. This can be useful when people delete zookeeper directory when zookeeper server shutdown. Typically useful on windows unit test


-- 
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: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] Shoothzj removed a comment on pull request #1766: ZOOKEEPER-4387 Close ZkDb when ZooKeeper Server shutdown

Posted by GitBox <gi...@apache.org>.
Shoothzj removed a comment on pull request #1766:
URL: https://github.com/apache/zookeeper/pull/1766#issuecomment-927252425


   @maoling @eolivelli PTAL, thanks


-- 
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: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] Shoothzj closed pull request #1766: ZOOKEEPER-4387 Close ZkDb when ZooKeeper Server shutdown

Posted by GitBox <gi...@apache.org>.
Shoothzj closed pull request #1766:
URL: https://github.com/apache/zookeeper/pull/1766


   


-- 
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: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] Shoothzj commented on pull request #1766: ZOOKEEPER-4387 Close ZkDb when ZooKeeper Server shutdown

Posted by GitBox <gi...@apache.org>.
Shoothzj commented on pull request #1766:
URL: https://github.com/apache/zookeeper/pull/1766#issuecomment-939925666


   @maoling could you please take a look :) 
   Thank you


-- 
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: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] Shoothzj commented on pull request #1766: ZOOKEEPER-4387 Close ZkDb when ZooKeeper Server shutdown

Posted by GitBox <gi...@apache.org>.
Shoothzj commented on pull request #1766:
URL: https://github.com/apache/zookeeper/pull/1766#issuecomment-940017190


   > so you are saying that we are never closing the ZK DB ?
   
   Yes, I found this problem while try to run pulsar unit tests on my windows pc. Can't delete the temporary directory


-- 
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: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] lvfangmin commented on pull request #1766: ZOOKEEPER-4387 Close ZkDb when ZooKeeper Server shutdown

Posted by GitBox <gi...@apache.org>.
lvfangmin commented on pull request #1766:
URL: https://github.com/apache/zookeeper/pull/1766#issuecomment-962115998


   ZkDatabase instance is reused, there is no `close` concept, it only clears the in memory data tree when fully shutdown is required. So the close function seems misleading, which seems closing the ZkDatabase instance, but actually it only closes the current streams in snapLog. 
   
   Also, the snapLog is initialized and passed into ZkDatabase, so the lifecycle is not tied to the ZkDatabase, ZkDatabase.close() to close the current open streams in snapLog seems a little strange.
   
   @Shoothzj for the unit test case, is there a way to close the snapLog directly instead of the zkDb? If not would suggest to rename the close() to a less misleading name at least.
   


-- 
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: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] Shoothzj commented on pull request #1766: ZOOKEEPER-4387 Close ZkDb when ZooKeeper Server shutdown

Posted by GitBox <gi...@apache.org>.
Shoothzj commented on pull request #1766:
URL: https://github.com/apache/zookeeper/pull/1766#issuecomment-950027331


   @maoling PTAL


-- 
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: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] lvfangmin commented on pull request #1766: ZOOKEEPER-4387 Close ZkDb when ZooKeeper Server shutdown

Posted by GitBox <gi...@apache.org>.
lvfangmin commented on pull request #1766:
URL: https://github.com/apache/zookeeper/pull/1766#issuecomment-962115998


   ZkDatabase instance is reused, there is no `close` concept, it only clears the in memory data tree when fully shutdown is required. So the close function seems misleading, which seems closing the ZkDatabase instance, but actually it only closes the current streams in snapLog. 
   
   Also, the snapLog is initialized and passed into ZkDatabase, so the lifecycle is not tied to the ZkDatabase, ZkDatabase.close() to close the current open streams in snapLog seems a little strange.
   
   @Shoothzj for the unit test case, is there a way to close the snapLog directly instead of the zkDb? If not would suggest to rename the close() to a less misleading name at least.
   


-- 
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: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] Shoothzj commented on pull request #1766: Close ZkDb when ZooKeeper Server shutdown

Posted by GitBox <gi...@apache.org>.
Shoothzj commented on pull request #1766:
URL: https://github.com/apache/zookeeper/pull/1766#issuecomment-927252425


   @maoling @eolivelli PTAL, thanks


-- 
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: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] eolivelli commented on pull request #1766: ZOOKEEPER-4387 Close ZkDb when ZooKeeper Server shutdown

Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #1766:
URL: https://github.com/apache/zookeeper/pull/1766#issuecomment-940013958


   so you are saying that we are never closing the ZK DB ?


-- 
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: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] lvfangmin commented on pull request #1766: ZOOKEEPER-4387 Close ZkDb when ZooKeeper Server shutdown

Posted by GitBox <gi...@apache.org>.
lvfangmin commented on pull request #1766:
URL: https://github.com/apache/zookeeper/pull/1766#issuecomment-962115998


   ZkDatabase instance is reused, there is no `close` concept, it only clears the in memory data tree when fully shutdown is required. So the close function seems misleading, which seems closing the ZkDatabase instance, but actually it only closes the current streams in snapLog. 
   
   Also, the snapLog is initialized and passed into ZkDatabase, so the lifecycle is not tied to the ZkDatabase, ZkDatabase.close() to close the current open streams in snapLog seems a little strange.
   
   @Shoothzj for the unit test case, is there a way to close the snapLog directly instead of the zkDb? If not would suggest to rename the close() to a less misleading name at least.
   


-- 
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: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] Shoothzj commented on pull request #1766: ZOOKEEPER-4387 Close ZkDb when ZooKeeper Server shutdown

Posted by GitBox <gi...@apache.org>.
Shoothzj commented on pull request #1766:
URL: https://github.com/apache/zookeeper/pull/1766#issuecomment-962510820


   > ZkDatabase instance is reused, there is no `close` concept, it only clears the in memory data tree when fully shutdown is required. So the close function seems misleading, which seems closing the ZkDatabase instance, but actually it only closes the current streams in snapLog.
   > 
   > Also, the snapLog is initialized and passed into ZkDatabase, so the lifecycle is not tied to the ZkDatabase, ZkDatabase.close() to close the current open streams in snapLog seems a little strange.
   > 
   > @Shoothzj for the unit test case, is there a way to close the snapLog directly instead of the zkDb? If not would suggest to rename the close() to a less misleading name at least.
   
   Thank you very much, I am doing this, and success. https://github.com/apache/pulsar/pull/12649/files


-- 
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: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] Shoothzj closed pull request #1766: ZOOKEEPER-4387 Close ZkDb when ZooKeeper Server shutdown

Posted by GitBox <gi...@apache.org>.
Shoothzj closed pull request #1766:
URL: https://github.com/apache/zookeeper/pull/1766


   


-- 
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: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org