You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2020/07/14 22:20:37 UTC
[GitHub] [geode] mhansonp opened a new pull request #5371: GEODE-8359
mhansonp opened a new pull request #5371:
URL: https://github.com/apache/geode/pull/5371
Fix to an NPE in RegisterInterestList66, needed to validate that a we were not getting a 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] [geode] mhansonp commented on a change in pull request #5371: GEODE-8359
Posted by GitBox <gi...@apache.org>.
mhansonp commented on a change in pull request #5371:
URL: https://github.com/apache/geode/pull/5371#discussion_r454830394
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterInterestList66.java
##########
@@ -163,29 +162,40 @@ public void cmdExecute(final Message clientMessage, final ServerConnection serve
}
try {
securityService.authorize(Resource.DATA, Operation.READ, regionName);
- AuthorizeRequest authzRequest = serverConnection.getAuthzRequest();
- if (authzRequest != null) {
+ AuthorizeRequest authorizeRequest = serverConnection.getAuthzRequest();
+ if (authorizeRequest != null) {
if (!DynamicRegionFactory.regionIsDynamicRegionList(regionName)) {
RegisterInterestOperationContext registerContext =
- authzRequest.registerInterestListAuthorize(regionName, keys, policy);
- keys = (List) registerContext.getKey();
+ authorizeRequest.registerInterestListAuthorize(regionName, keys, policy);
+ keys = (List<Object>) registerContext.getKey();
}
}
// Register interest
serverConnection.getAcceptor().getCacheClientNotifier().registerClientInterest(regionName,
keys, serverConnection.getProxyID(), isDurable, sendUpdatesAsInvalidates, true,
regionDataPolicyPartBytes[0], true);
- } catch (Exception ex) {
+ } catch (Exception e) {
// If an interrupted exception is thrown , rethrow it
- checkForInterrupt(serverConnection, ex);
+ checkForInterrupt(serverConnection, e);
// Otherwise, write an exception message and continue
- writeChunkedException(clientMessage, ex, serverConnection);
+ writeChunkedException(clientMessage, e, serverConnection);
serverConnection.setAsTrue(RESPONDED);
return;
}
- boolean isPrimary = serverConnection.getAcceptor().getCacheClientNotifier()
- .getClientProxy(serverConnection.getProxyID()).isPrimary();
+ CacheClientProxy ccp = serverConnection.getAcceptor().getCacheClientNotifier()
+ .getClientProxy(serverConnection.getProxyID());
+
+ if (ccp == null) {
Review comment:
The important chunk here copied from RegisterInterest61 which already had a bug fix for this issue.
----------------------------------------------------------------
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] [geode] mhansonp merged pull request #5371: GEODE-8359
Posted by GitBox <gi...@apache.org>.
mhansonp merged pull request #5371:
URL: https://github.com/apache/geode/pull/5371
----------------------------------------------------------------
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] [geode] mhansonp commented on a change in pull request #5371: GEODE-8359
Posted by GitBox <gi...@apache.org>.
mhansonp commented on a change in pull request #5371:
URL: https://github.com/apache/geode/pull/5371#discussion_r454830735
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterInterestList66.java
##########
@@ -163,29 +162,40 @@ public void cmdExecute(final Message clientMessage, final ServerConnection serve
}
try {
securityService.authorize(Resource.DATA, Operation.READ, regionName);
- AuthorizeRequest authzRequest = serverConnection.getAuthzRequest();
- if (authzRequest != null) {
+ AuthorizeRequest authorizeRequest = serverConnection.getAuthzRequest();
+ if (authorizeRequest != null) {
if (!DynamicRegionFactory.regionIsDynamicRegionList(regionName)) {
RegisterInterestOperationContext registerContext =
- authzRequest.registerInterestListAuthorize(regionName, keys, policy);
- keys = (List) registerContext.getKey();
+ authorizeRequest.registerInterestListAuthorize(regionName, keys, policy);
+ keys = (List<Object>) registerContext.getKey();
}
}
// Register interest
serverConnection.getAcceptor().getCacheClientNotifier().registerClientInterest(regionName,
keys, serverConnection.getProxyID(), isDurable, sendUpdatesAsInvalidates, true,
regionDataPolicyPartBytes[0], true);
- } catch (Exception ex) {
+ } catch (Exception e) {
// If an interrupted exception is thrown , rethrow it
- checkForInterrupt(serverConnection, ex);
+ checkForInterrupt(serverConnection, e);
// Otherwise, write an exception message and continue
- writeChunkedException(clientMessage, ex, serverConnection);
+ writeChunkedException(clientMessage, e, serverConnection);
serverConnection.setAsTrue(RESPONDED);
return;
}
- boolean isPrimary = serverConnection.getAcceptor().getCacheClientNotifier()
- .getClientProxy(serverConnection.getProxyID()).isPrimary();
+ CacheClientProxy ccp = serverConnection.getAcceptor().getCacheClientNotifier()
+ .getClientProxy(serverConnection.getProxyID());
+
+ if (ccp == null) {
Review comment:
The core issue was an NPE, because there was no cache client proxy returned during shutdown.
----------------------------------------------------------------
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] [geode] mhansonp commented on a change in pull request #5371: GEODE-8359
Posted by GitBox <gi...@apache.org>.
mhansonp commented on a change in pull request #5371:
URL: https://github.com/apache/geode/pull/5371#discussion_r455924506
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterInterestList66.java
##########
@@ -163,29 +162,40 @@ public void cmdExecute(final Message clientMessage, final ServerConnection serve
}
try {
securityService.authorize(Resource.DATA, Operation.READ, regionName);
- AuthorizeRequest authzRequest = serverConnection.getAuthzRequest();
- if (authzRequest != null) {
+ AuthorizeRequest authorizeRequest = serverConnection.getAuthzRequest();
+ if (authorizeRequest != null) {
if (!DynamicRegionFactory.regionIsDynamicRegionList(regionName)) {
RegisterInterestOperationContext registerContext =
- authzRequest.registerInterestListAuthorize(regionName, keys, policy);
- keys = (List) registerContext.getKey();
+ authorizeRequest.registerInterestListAuthorize(regionName, keys, policy);
+ keys = (List<Object>) registerContext.getKey();
}
}
// Register interest
serverConnection.getAcceptor().getCacheClientNotifier().registerClientInterest(regionName,
keys, serverConnection.getProxyID(), isDurable, sendUpdatesAsInvalidates, true,
regionDataPolicyPartBytes[0], true);
- } catch (Exception ex) {
+ } catch (Exception e) {
// If an interrupted exception is thrown , rethrow it
- checkForInterrupt(serverConnection, ex);
+ checkForInterrupt(serverConnection, e);
// Otherwise, write an exception message and continue
- writeChunkedException(clientMessage, ex, serverConnection);
+ writeChunkedException(clientMessage, e, serverConnection);
serverConnection.setAsTrue(RESPONDED);
return;
}
- boolean isPrimary = serverConnection.getAcceptor().getCacheClientNotifier()
- .getClientProxy(serverConnection.getProxyID()).isPrimary();
+ CacheClientProxy ccp = serverConnection.getAcceptor().getCacheClientNotifier()
+ .getClientProxy(serverConnection.getProxyID());
+
+ if (ccp == null) {
+ IOException ioException = new IOException(
+ "CacheClientProxy for this client is no longer on the server , so registerInterest operation is unsuccessful");
Review comment:
done
----------------------------------------------------------------
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] [geode] agingade commented on a change in pull request #5371: GEODE-8359
Posted by GitBox <gi...@apache.org>.
agingade commented on a change in pull request #5371:
URL: https://github.com/apache/geode/pull/5371#discussion_r455242606
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterInterestList66.java
##########
@@ -163,29 +162,40 @@ public void cmdExecute(final Message clientMessage, final ServerConnection serve
}
try {
securityService.authorize(Resource.DATA, Operation.READ, regionName);
- AuthorizeRequest authzRequest = serverConnection.getAuthzRequest();
- if (authzRequest != null) {
+ AuthorizeRequest authorizeRequest = serverConnection.getAuthzRequest();
+ if (authorizeRequest != null) {
if (!DynamicRegionFactory.regionIsDynamicRegionList(regionName)) {
RegisterInterestOperationContext registerContext =
- authzRequest.registerInterestListAuthorize(regionName, keys, policy);
- keys = (List) registerContext.getKey();
+ authorizeRequest.registerInterestListAuthorize(regionName, keys, policy);
+ keys = (List<Object>) registerContext.getKey();
}
}
// Register interest
serverConnection.getAcceptor().getCacheClientNotifier().registerClientInterest(regionName,
keys, serverConnection.getProxyID(), isDurable, sendUpdatesAsInvalidates, true,
regionDataPolicyPartBytes[0], true);
- } catch (Exception ex) {
+ } catch (Exception e) {
// If an interrupted exception is thrown , rethrow it
- checkForInterrupt(serverConnection, ex);
+ checkForInterrupt(serverConnection, e);
// Otherwise, write an exception message and continue
- writeChunkedException(clientMessage, ex, serverConnection);
+ writeChunkedException(clientMessage, e, serverConnection);
serverConnection.setAsTrue(RESPONDED);
return;
}
- boolean isPrimary = serverConnection.getAcceptor().getCacheClientNotifier()
- .getClientProxy(serverConnection.getProxyID()).isPrimary();
+ CacheClientProxy ccp = serverConnection.getAcceptor().getCacheClientNotifier()
+ .getClientProxy(serverConnection.getProxyID());
+
+ if (ccp == null) {
+ IOException ioException = new IOException(
+ "CacheClientProxy for this client is no longer on the server , so registerInterest operation is unsuccessful");
Review comment:
We could change the messaging part...
----------------------------------------------------------------
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