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 2020/09/01 11:23:41 UTC
[GitHub] [zookeeper] yanllearnn opened a new pull request #1443: ZOOKEEPER-3916: if zkServer is null, don't accept connection for wasting resource, until not null
yanllearnn opened a new pull request #1443:
URL: https://github.com/apache/zookeeper/pull/1443
## when leader is shutdown or not voted yet
```
Leader#shutdown ---> self.setZooKeeperServer(null) ---> NIOServerCnxnFactory.SelectorThread#run
---> processAcceptedConnections ---> createConnection(zkServer=null)
```
## when the client tries to connect that uninitialized server
* origin, accept the connections immediately. When reading data check whether the zkServer is null or not. If null, throws exceptions and closes connections, which wastes the resources.
```
NIOServerCnxn#doIO---> NIOServerCnxn#readLength --->
if (!isZKServerRunning()) ---> throw new IOException("ZooKeeperServer not running")
```
* current, precheck whether the zkServer is null or not. If null, not accept the connetion.
```
QuorumPeerMain#quorumPeer.start();--->QuorumPeer#startServerCnxnFactory();--->ServerCnxnFactory#start();--->
acceptThread.start(); then thread is running waiting for connection to accepted;
--->AcceptThread#run, if (zkServer == null) continue; don't accept the connection, until zkServer is not null
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] yanllearnn commented on pull request #1443: ZOOKEEPER-3916: if zkServer is null, don't accept connection for wasting resource, until not null
Posted by GitBox <gi...@apache.org>.
yanllearnn commented on pull request #1443:
URL: https://github.com/apache/zookeeper/pull/1443#issuecomment-686233464
@eolivelli
I have added the sleep and remove the second check
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] yanllearnn commented on a change in pull request #1443: ZOOKEEPER-3916: if zkServer is null, don't accept connection for wasting resource, until not null
Posted by GitBox <gi...@apache.org>.
yanllearnn commented on a change in pull request #1443:
URL: https://github.com/apache/zookeeper/pull/1443#discussion_r481068462
##########
File path: zookeeper-server/src/main/java/org/apache/zookeeper/server/NIOServerCnxnFactory.java
##########
@@ -820,7 +828,12 @@ private void addCnxn(NIOServerCnxn cnxn) throws IOException {
}
protected NIOServerCnxn createConnection(SocketChannel sock, SelectionKey sk, SelectorThread selectorThread) throws IOException {
- return new NIOServerCnxn(zkServer, sock, sk, this, selectorThread);
+ if (zkServer != null) {
Review comment:
@eolivelli
here is not required, but can check again. So I think adding it is well
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] yanllearnn commented on a change in pull request #1443: ZOOKEEPER-3916: if zkServer is null, don't accept connection for wasting resource, until not null
Posted by GitBox <gi...@apache.org>.
yanllearnn commented on a change in pull request #1443:
URL: https://github.com/apache/zookeeper/pull/1443#discussion_r481069531
##########
File path: zookeeper-server/src/main/java/org/apache/zookeeper/server/NIOServerCnxnFactory.java
##########
@@ -178,6 +178,14 @@ public void run() {
try {
while (!stopped && !acceptSocket.socket().isClosed()) {
try {
+ /** if zkServer is null, don't accept the connection, until not null.
+ * if accept the connection, when reading data from socket, it will be closed by null of zkServer
+ *
+ */
+ if (zkServer == null) {
Review comment:
@eolivelli
No need. The loop is ok. If sleep a little time, maybe cause the clients waiting to connect to servers.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] eolivelli commented on a change in pull request #1443: ZOOKEEPER-3916: if zkServer is null, don't accept connection for wasting resource, until not null
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #1443:
URL: https://github.com/apache/zookeeper/pull/1443#discussion_r481066477
##########
File path: zookeeper-server/src/main/java/org/apache/zookeeper/server/NIOServerCnxnFactory.java
##########
@@ -178,6 +178,14 @@ public void run() {
try {
while (!stopped && !acceptSocket.socket().isClosed()) {
try {
+ /** if zkServer is null, don't accept the connection, until not null.
+ * if accept the connection, when reading data from socket, it will be closed by null of zkServer
+ *
+ */
+ if (zkServer == null) {
Review comment:
shall we sleep a little time ?
otherwise we are in a busy loop
##########
File path: zookeeper-server/src/main/java/org/apache/zookeeper/server/NIOServerCnxnFactory.java
##########
@@ -820,7 +828,12 @@ private void addCnxn(NIOServerCnxn cnxn) throws IOException {
}
protected NIOServerCnxn createConnection(SocketChannel sock, SelectionKey sk, SelectorThread selectorThread) throws IOException {
- return new NIOServerCnxn(zkServer, sock, sk, this, selectorThread);
+ if (zkServer != null) {
Review comment:
given your fix above, is this check still useful ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] yanllearnn closed pull request #1443: ZOOKEEPER-3916: if zkServer is null, don't accept connection for wasting resource, until not null
Posted by GitBox <gi...@apache.org>.
yanllearnn closed pull request #1443:
URL: https://github.com/apache/zookeeper/pull/1443
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org