[jira] [Updated] (GEODE-8277) acceptance test certificates expired in Dockerized SNI acceptance tests
[
https://issues.apache.org/jira/browse/GEODE-8277?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Bill Burcham updated GEODE-8277:
Description:
At least locator-maeve-keystore.jks is expired. This is causing CI failures now.
All the certs: the CA cert, and each of the three server certs are expired.
These need to all be regenerated using {{CertificateBuilder}} and friends.
I found neither instructions nor a script for regenerating these keystores and
truststores. They may be there but I didn't find 'em.
TODO:
# regenerate signing (CA) cert in truststore.jks
# create the three new keystores containing certs signed by the signing cert
# make sure all the expirations are at least 10 years hence
# make sure there is a README or better: a README and a script for generating
all that
# make sure all the SNI acceptance tests pass locally {{./gradlew
:geode-assembly:acceptanceTest --tests=org.apache.geode.client.sni.*}}
was:
At least locator-maeve-keystore.jks is expired. This is causing CI failures now.
All the certs: the CA cert, and each of the three server certs are expired.
These need to all be regenerated using {{CertificateBuilder}} and friends.
I found neither instructions nor a script for regenerating these keystores and
truststores. They may be there but I didn't find 'em.
> acceptance test certificates expired in Dockerized SNI acceptance tests
> ---
>
> Key: GEODE-8277
> URL: https://issues.apache.org/jira/browse/GEODE-8277
> Project: Geode
> Issue Type: Bug
> Components: tests
>Reporter: Bill Burcham
>Assignee: Bruce J Schuchardt
>Priority: Major
>
> At least locator-maeve-keystore.jks is expired. This is causing CI failures
> now.
> All the certs: the CA cert, and each of the three server certs are expired.
> These need to all be regenerated using {{CertificateBuilder}} and friends.
> I found neither instructions nor a script for regenerating these keystores
> and truststores. They may be there but I didn't find 'em.
> TODO:
> # regenerate signing (CA) cert in truststore.jks
> # create the three new keystores containing certs signed by the signing cert
> # make sure all the expirations are at least 10 years hence
> # make sure there is a README or better: a README and a script for generating
> all that
> # make sure all the SNI acceptance tests pass locally {{./gradlew
> :geode-assembly:acceptanceTest --tests=org.apache.geode.client.sni.*}}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Updated] (GEODE-8277) acceptance test certificates expired in Dockerized SNI acceptance tests
[
https://issues.apache.org/jira/browse/GEODE-8277?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Bill Burcham updated GEODE-8277:
Description:
At least locator-maeve-keystore.jks is expired. This is causing CI failures now.
All the certs: the CA cert, and each of the three server certs are expired.
These need to all be regenerated using {{CertificateBuilder}} and friends.
I found neither instructions nor a script for regenerating these keystores and
truststores. They may be there but I didn't find 'em.
was:
At least locator-maeve-keystore.jks is expired. This is causing CI failures now.
All the certs: the CA cert, and each of the three server certs are expired.
These need to all be regenerated using {{CertificateBuilder}} and friends.
> acceptance test certificates expired in Dockerized SNI acceptance tests
> ---
>
> Key: GEODE-8277
> URL: https://issues.apache.org/jira/browse/GEODE-8277
> Project: Geode
> Issue Type: Bug
> Components: tests
>Reporter: Bill Burcham
>Assignee: Bruce J Schuchardt
>Priority: Major
>
> At least locator-maeve-keystore.jks is expired. This is causing CI failures
> now.
> All the certs: the CA cert, and each of the three server certs are expired.
> These need to all be regenerated using {{CertificateBuilder}} and friends.
> I found neither instructions nor a script for regenerating these keystores
> and truststores. They may be there but I didn't find 'em.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Updated] (GEODE-8277) acceptance test certificates expired in Dockerized SNI acceptance tests
[
https://issues.apache.org/jira/browse/GEODE-8277?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Bill Burcham updated GEODE-8277:
Description:
At least locator-maeve-keystore.jks is expired. This is causing CI failures now.
All the certs: the CA cert, and each of the three server certs are expired.
These need to all be regenerated using {{CertificateBuilder}} and friends.
was:At least locator-maeve-keystore.jks is expired. This is causing CI
failures now.
> acceptance test certificates expired in Dockerized SNI acceptance tests
> ---
>
> Key: GEODE-8277
> URL: https://issues.apache.org/jira/browse/GEODE-8277
> Project: Geode
> Issue Type: Bug
> Components: tests
>Reporter: Bill Burcham
>Assignee: Bill Burcham
>Priority: Major
>
> At least locator-maeve-keystore.jks is expired. This is causing CI failures
> now.
> All the certs: the CA cert, and each of the three server certs are expired.
> These need to all be regenerated using {{CertificateBuilder}} and friends.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (GEODE-8277) acceptance test certificates expired in Dockerized SNI acceptance tests
[
https://issues.apache.org/jira/browse/GEODE-8277?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138956#comment-17138956
]
Bill Burcham commented on GEODE-8277:
-
Ugh. My manual cert-building style is insufficient. Seeing:
{code}
Caused by: java.security.cert.CertificateException: No subject alternative
names present
{code}
in the locator log. My approach won't work. We need to use the Java program to
regenerate these.
> acceptance test certificates expired in Dockerized SNI acceptance tests
> ---
>
> Key: GEODE-8277
> URL: https://issues.apache.org/jira/browse/GEODE-8277
> Project: Geode
> Issue Type: Bug
> Components: tests
>Reporter: Bill Burcham
>Assignee: Bill Burcham
>Priority: Major
>
> At least locator-maeve-keystore.jks is expired. This is causing CI failures
> now.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (GEODE-8277) acceptance test certificates expired in Dockerized SNI acceptance tests
[ https://issues.apache.org/jira/browse/GEODE-8277?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138947#comment-17138947 ] Bill Burcham commented on GEODE-8277: - I'm working on a fix for this. The problem is that when we used Sai's Java program to generate the certs last month, we set the expiration to only one month hence (now!) I looked at the generated keystores and truststore. The setup is good in that the truststore contains only the CA cert and the keystores each contain one cert signed by that CA. Unfortunately I don't know how to use Sai's Java program to recreate these with extended expiration so I am trying to make do with the macOS KeyStore Explorer app. I manually created three keystores each containing a self-signed cert (expiring in 10 years) and added all three to the truststore. Running tests now. Fingers crossed 爛 > acceptance test certificates expired in Dockerized SNI acceptance tests > --- > > Key: GEODE-8277 > URL: https://issues.apache.org/jira/browse/GEODE-8277 > Project: Geode > Issue Type: Bug > Components: tests >Reporter: Bill Burcham >Assignee: Bill Burcham >Priority: Major > > At least locator-maeve-keystore.jks is expired. This is causing CI failures > now. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Assigned] (GEODE-8277) acceptance test certificates expired in Dockerized SNI acceptance tests
[ https://issues.apache.org/jira/browse/GEODE-8277?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Bill Burcham reassigned GEODE-8277: --- Assignee: Bill Burcham > acceptance test certificates expired in Dockerized SNI acceptance tests > --- > > Key: GEODE-8277 > URL: https://issues.apache.org/jira/browse/GEODE-8277 > Project: Geode > Issue Type: Bug > Components: tests >Reporter: Bill Burcham >Assignee: Bill Burcham >Priority: Major > > At least locator-maeve-keystore.jks is expired. This is causing CI failures > now. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-8263) Redis SET command should not accept the KEEPTTL option
[ https://issues.apache.org/jira/browse/GEODE-8263?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138907#comment-17138907 ] ASF subversion and git services commented on GEODE-8263: Commit 9fdd3d03c97dbe6521d330b5297f0a053030fe26 in geode's branch refs/heads/develop from Darrel Schneider [ https://gitbox.apache.org/repos/asf?p=geode.git;h=9fdd3d0 ] GEODE-8263: change SET command to reject KEEPTTL (#5258) > Redis SET command should not accept the KEEPTTL option > -- > > Key: GEODE-8263 > URL: https://issues.apache.org/jira/browse/GEODE-8263 > Project: Geode > Issue Type: Bug > Components: redis >Reporter: Darrel Schneider >Assignee: Darrel Schneider >Priority: Major > Fix For: 1.14.0 > > > The Redis SET commands should not accept the KEEPTTL option because geode > redis currently implements redis 5 and KEEPTTL is part of redis 6. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-8263) Redis SET command should not accept the KEEPTTL option
[ https://issues.apache.org/jira/browse/GEODE-8263?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138908#comment-17138908 ] ASF subversion and git services commented on GEODE-8263: Commit 9fdd3d03c97dbe6521d330b5297f0a053030fe26 in geode's branch refs/heads/develop from Darrel Schneider [ https://gitbox.apache.org/repos/asf?p=geode.git;h=9fdd3d0 ] GEODE-8263: change SET command to reject KEEPTTL (#5258) > Redis SET command should not accept the KEEPTTL option > -- > > Key: GEODE-8263 > URL: https://issues.apache.org/jira/browse/GEODE-8263 > Project: Geode > Issue Type: Bug > Components: redis >Reporter: Darrel Schneider >Assignee: Darrel Schneider >Priority: Major > Fix For: 1.14.0 > > > The Redis SET commands should not accept the KEEPTTL option because geode > redis currently implements redis 5 and KEEPTTL is part of redis 6. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-8263) Redis SET command should not accept the KEEPTTL option
[ https://issues.apache.org/jira/browse/GEODE-8263?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138906#comment-17138906 ] ASF GitHub Bot commented on GEODE-8263: --- dschneider-pivotal merged pull request #5258: URL: https://github.com/apache/geode/pull/5258 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: us...@infra.apache.org > Redis SET command should not accept the KEEPTTL option > -- > > Key: GEODE-8263 > URL: https://issues.apache.org/jira/browse/GEODE-8263 > Project: Geode > Issue Type: Bug > Components: redis >Reporter: Darrel Schneider >Assignee: Darrel Schneider >Priority: Major > > The Redis SET commands should not accept the KEEPTTL option because geode > redis currently implements redis 5 and KEEPTTL is part of redis 6. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Resolved] (GEODE-8263) Redis SET command should not accept the KEEPTTL option
[ https://issues.apache.org/jira/browse/GEODE-8263?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Darrel Schneider resolved GEODE-8263. - Fix Version/s: 1.14.0 Resolution: Fixed > Redis SET command should not accept the KEEPTTL option > -- > > Key: GEODE-8263 > URL: https://issues.apache.org/jira/browse/GEODE-8263 > Project: Geode > Issue Type: Bug > Components: redis >Reporter: Darrel Schneider >Assignee: Darrel Schneider >Priority: Major > Fix For: 1.14.0 > > > The Redis SET commands should not accept the KEEPTTL option because geode > redis currently implements redis 5 and KEEPTTL is part of redis 6. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-8263) Redis SET command should not accept the KEEPTTL option
[ https://issues.apache.org/jira/browse/GEODE-8263?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138904#comment-17138904 ] ASF GitHub Bot commented on GEODE-8263: --- dschneider-pivotal commented on pull request #5258: URL: https://github.com/apache/geode/pull/5258#issuecomment-645685812 the dunit failure is a known flaky 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Redis SET command should not accept the KEEPTTL option > -- > > Key: GEODE-8263 > URL: https://issues.apache.org/jira/browse/GEODE-8263 > Project: Geode > Issue Type: Bug > Components: redis >Reporter: Darrel Schneider >Assignee: Darrel Schneider >Priority: Major > > The Redis SET commands should not accept the KEEPTTL option because geode > redis currently implements redis 5 and KEEPTTL is part of redis 6. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-8272) Refactor GFSH Restore Redundancy Command code to allow for REST API
[ https://issues.apache.org/jira/browse/GEODE-8272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138889#comment-17138889 ] ASF GitHub Bot commented on GEODE-8272: --- mhansonp commented on a change in pull request #5249: URL: https://github.com/apache/geode/pull/5249#discussion_r441876791 ## File path: geode-core/src/main/java/org/apache/geode/management/internal/operation/RestoreRedundancyPerformer.java ## @@ -112,14 +114,14 @@ public RestoreRedundancyResults perform(Cache cache, RestoreRedundancyRequest op return finalResult; } - // this returns either an Exception or RestoreRedundancyResults + // this returns RestoreRedundancyResults or null based on Review comment: Good catch. Task switching is bad. The comment just restates what is visible in the code so I deleted it. 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: us...@infra.apache.org > Refactor GFSH Restore Redundancy Command code to allow for REST API > --- > > Key: GEODE-8272 > URL: https://issues.apache.org/jira/browse/GEODE-8272 > Project: Geode > Issue Type: Bug > Components: gfsh >Reporter: Mark Hanson >Assignee: Mark Hanson >Priority: Major > > Refactor GFSH Restore Redundancy Command code to allow for REST API > > The core code path is specific to GFSH right now and needs a few changes to > support the REST API command. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-8272) Refactor GFSH Restore Redundancy Command code to allow for REST API
[ https://issues.apache.org/jira/browse/GEODE-8272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138890#comment-17138890 ] ASF GitHub Bot commented on GEODE-8272: --- mhansonp commented on a change in pull request #5249: URL: https://github.com/apache/geode/pull/5249#discussion_r441876860 ## File path: geode-core/src/main/java/org/apache/geode/management/internal/operation/RestoreRedundancyPerformer.java ## @@ -112,14 +114,14 @@ public RestoreRedundancyResults perform(Cache cache, RestoreRedundancyRequest op return finalResult; } - // this returns either an Exception or RestoreRedundancyResults + // this returns RestoreRedundancyResults or null based on Review comment: Thanks for your attention to detail. 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: us...@infra.apache.org > Refactor GFSH Restore Redundancy Command code to allow for REST API > --- > > Key: GEODE-8272 > URL: https://issues.apache.org/jira/browse/GEODE-8272 > Project: Geode > Issue Type: Bug > Components: gfsh >Reporter: Mark Hanson >Assignee: Mark Hanson >Priority: Major > > Refactor GFSH Restore Redundancy Command code to allow for REST API > > The core code path is specific to GFSH right now and needs a few changes to > support the REST API command. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-8272) Refactor GFSH Restore Redundancy Command code to allow for REST API
[
https://issues.apache.org/jira/browse/GEODE-8272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=1713#comment-1713
]
ASF GitHub Bot commented on GEODE-8272:
---
mhansonp commented on a change in pull request #5249:
URL: https://github.com/apache/geode/pull/5249#discussion_r441875211
##
File path:
geode-management/src/main/java/org/apache/geode/management/internal/operation/RegionRedundancyStatusImpl.java
##
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express
+ * or implied. See the License for the specific language governing permissions
and limitations under
+ * the License.
+ */
+package org.apache.geode.management.internal.operation;
+
+import org.apache.geode.management.runtime.RegionRedundancyStatus;
+
+/**
+ * result object used by the cms that only needs to be json serializable
+ */
+public class RegionRedundancyStatusImpl implements RegionRedundancyStatus {
+
+ public static final String OUTPUT_STRING =
+ "%s redundancy status: %s. Desired redundancy is %s and actual
redundancy is %s.";
+
+ /**
+ * The name of the region used to create this object.
+ */
+ protected String regionName;
+
+ /**
+ * The configured redundancy of the region used to create this object.
+ */
+ protected int configuredRedundancy;
+
+ /**
+ * The actual redundancy of the region used to create this object at time of
creation.
+ */
+ protected int actualRedundancy;
+
+ /**
+ * The {@link RedundancyStatus} of the region used to create this object at
time of creation.
+ */
+ protected RedundancyStatus status;
+
+ /**
+ * Default constructor used for serialization
+ */
+ public RegionRedundancyStatusImpl() {}
+
+ public RegionRedundancyStatusImpl(int configuredRedundancy, int
actualRedundancy,
Review comment:
Sorry misread that... I agree.
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:
us...@infra.apache.org
> Refactor GFSH Restore Redundancy Command code to allow for REST API
> ---
>
> Key: GEODE-8272
> URL: https://issues.apache.org/jira/browse/GEODE-8272
> Project: Geode
> Issue Type: Bug
> Components: gfsh
>Reporter: Mark Hanson
>Assignee: Mark Hanson
>Priority: Major
>
> Refactor GFSH Restore Redundancy Command code to allow for REST API
>
> The core code path is specific to GFSH right now and needs a few changes to
> support the REST API command.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (GEODE-8272) Refactor GFSH Restore Redundancy Command code to allow for REST API
[
https://issues.apache.org/jira/browse/GEODE-8272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138887#comment-17138887
]
ASF GitHub Bot commented on GEODE-8272:
---
mhansonp commented on a change in pull request #5249:
URL: https://github.com/apache/geode/pull/5249#discussion_r441874934
##
File path:
geode-management/src/main/java/org/apache/geode/management/internal/operation/RegionRedundancyStatusImpl.java
##
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express
+ * or implied. See the License for the specific language governing permissions
and limitations under
+ * the License.
+ */
+package org.apache.geode.management.internal.operation;
+
+import org.apache.geode.management.runtime.RegionRedundancyStatus;
+
+/**
+ * result object used by the cms that only needs to be json serializable
+ */
+public class RegionRedundancyStatusImpl implements RegionRedundancyStatus {
+
+ public static final String OUTPUT_STRING =
+ "%s redundancy status: %s. Desired redundancy is %s and actual
redundancy is %s.";
+
+ /**
+ * The name of the region used to create this object.
+ */
+ protected String regionName;
+
+ /**
+ * The configured redundancy of the region used to create this object.
+ */
+ protected int configuredRedundancy;
+
+ /**
+ * The actual redundancy of the region used to create this object at time of
creation.
+ */
+ protected int actualRedundancy;
+
+ /**
+ * The {@link RedundancyStatus} of the region used to create this object at
time of creation.
+ */
+ protected RedundancyStatus status;
+
+ /**
+ * Default constructor used for serialization
+ */
+ public RegionRedundancyStatusImpl() {}
+
+ public RegionRedundancyStatusImpl(int configuredRedundancy, int
actualRedundancy,
Review comment:
Its necessary for serialization.
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:
us...@infra.apache.org
> Refactor GFSH Restore Redundancy Command code to allow for REST API
> ---
>
> Key: GEODE-8272
> URL: https://issues.apache.org/jira/browse/GEODE-8272
> Project: Geode
> Issue Type: Bug
> Components: gfsh
>Reporter: Mark Hanson
>Assignee: Mark Hanson
>Priority: Major
>
> Refactor GFSH Restore Redundancy Command code to allow for REST API
>
> The core code path is specific to GFSH right now and needs a few changes to
> support the REST API command.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (GEODE-8272) Refactor GFSH Restore Redundancy Command code to allow for REST API
[ https://issues.apache.org/jira/browse/GEODE-8272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138885#comment-17138885 ] ASF GitHub Bot commented on GEODE-8272: --- DonalEvans commented on a change in pull request #5249: URL: https://github.com/apache/geode/pull/5249#discussion_r441873906 ## File path: geode-core/src/main/java/org/apache/geode/management/internal/operation/RestoreRedundancyPerformer.java ## @@ -112,14 +114,14 @@ public RestoreRedundancyResults perform(Cache cache, RestoreRedundancyRequest op return finalResult; } - // this returns either an Exception or RestoreRedundancyResults + // this returns RestoreRedundancyResults or null based on Review comment: Incomplete comment here? 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: us...@infra.apache.org > Refactor GFSH Restore Redundancy Command code to allow for REST API > --- > > Key: GEODE-8272 > URL: https://issues.apache.org/jira/browse/GEODE-8272 > Project: Geode > Issue Type: Bug > Components: gfsh >Reporter: Mark Hanson >Assignee: Mark Hanson >Priority: Major > > Refactor GFSH Restore Redundancy Command code to allow for REST API > > The core code path is specific to GFSH right now and needs a few changes to > support the REST API command. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-8272) Refactor GFSH Restore Redundancy Command code to allow for REST API
[
https://issues.apache.org/jira/browse/GEODE-8272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138883#comment-17138883
]
ASF GitHub Bot commented on GEODE-8272:
---
mhansonp commented on a change in pull request #5249:
URL: https://github.com/apache/geode/pull/5249#discussion_r441871215
##
File path:
geode-core/src/test/java/org/apache/geode/management/internal/functions/RestoreRedundancyFunctionTest.java
##
@@ -0,0 +1,163 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express
+ * or implied. See the License for the specific language governing permissions
and limitations under
+ * the License.
+ */
+
+package org.apache.geode.management.internal.functions;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.control.RestoreRedundancyOperation;
+import org.apache.geode.cache.execute.FunctionContext;
+import org.apache.geode.cache.execute.ResultSender;
+import
org.apache.geode.internal.cache.control.SerializableRestoreRedundancyResultsImpl;
+import org.apache.geode.management.operation.RestoreRedundancyRequest;
+import org.apache.geode.management.runtime.RestoreRedundancyResults;
+
+public class RestoreRedundancyFunctionTest {
+ @SuppressWarnings("unchecked")
+ private final FunctionContext mockContext =
mock(FunctionContext.class);
+ private final Cache mockCache = mock(Cache.class, RETURNS_DEEP_STUBS);
+ private final RestoreRedundancyOperation mockOperation =
+ mock(RestoreRedundancyOperation.class, RETURNS_DEEP_STUBS);
+ private final SerializableRestoreRedundancyResultsImpl mockResults =
+ mock(SerializableRestoreRedundancyResultsImpl.class);
+ private final String message = "expected message";
+ private RestoreRedundancyFunction function;
+ private ResultSender resultSender;
+ private ArgumentCaptor
argumentCaptor;
+ private RestoreRedundancyRequest request;
+
+ @Before
+ public void setUp() throws InterruptedException, ExecutionException {
+function = new RestoreRedundancyFunction();
+when(mockContext.getCache()).thenReturn(mockCache);
+request = new RestoreRedundancyRequest();
Review comment:
I think it is reasonable at this point to believe that. It is basically
like a work order being passed in...
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:
us...@infra.apache.org
> Refactor GFSH Restore Redundancy Command code to allow for REST API
> ---
>
> Key: GEODE-8272
> URL: https://issues.apache.org/jira/browse/GEODE-8272
> Project: Geode
> Issue Type: Bug
> Components: gfsh
>Reporter: Mark Hanson
>Assignee: Mark Hanson
>Priority: Major
>
> Refactor GFSH Restore Redundancy Command code to allow for REST API
>
> The core code path is specific to GFSH right now and needs a few changes to
> support the REST API command.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (GEODE-8272) Refactor GFSH Restore Redundancy Command code to allow for REST API
[
https://issues.apache.org/jira/browse/GEODE-8272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138876#comment-17138876
]
ASF GitHub Bot commented on GEODE-8272:
---
mhansonp commented on a change in pull request #5249:
URL: https://github.com/apache/geode/pull/5249#discussion_r441868918
##
File path:
geode-core/src/main/java/org/apache/geode/management/internal/operation/RestoreRedundancyPerformer.java
##
@@ -0,0 +1,179 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express
+ * or implied. See the License for the specific language governing permissions
and limitations under
+ * the License.
+ */
+
+package org.apache.geode.management.internal.operation;
+
+import static org.apache.geode.cache.Region.SEPARATOR;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.stream.Collectors;
+
+import org.apache.geode.annotations.Immutable;
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.execute.Function;
+import org.apache.geode.cache.execute.ResultCollector;
+import org.apache.geode.distributed.DistributedMember;
+import
org.apache.geode.distributed.internal.membership.InternalDistributedMember;
+import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.internal.serialization.Version;
+import org.apache.geode.management.ManagementService;
+import
org.apache.geode.management.internal.functions.RestoreRedundancyFunction;
+import org.apache.geode.management.internal.util.ManagementUtils;
+import org.apache.geode.management.operation.RestoreRedundancyRequest;
+import org.apache.geode.management.runtime.RestoreRedundancyResults;
+
+public class RestoreRedundancyPerformer
+implements OperationPerformer {
+ @Immutable
+ public static final Version ADDED_VERSION = Version.GEODE_1_13_0;
+ public static final String NO_MEMBERS_WITH_VERSION_FOR_REGION =
+ "No members with a version greater than or equal to %s were found for
region %s";
+ public static final String EXCEPTION_MEMBER_MESSAGE = "Exception occurred on
member %s: %s";
+
+ @Override
+ public RestoreRedundancyResults perform(Cache cache,
RestoreRedundancyRequest operation) {
+return perform(cache, operation, false);
+ }
+
+ public RestoreRedundancyResults perform(Cache cache,
RestoreRedundancyRequest operation,
+ boolean checkStatus) {
+List membersForEachRegion = new
ArrayList<>();
+List includedRegionsWithNoMembers = new ArrayList<>();
+
+populateLists(membersForEachRegion, includedRegionsWithNoMembers,
operation.getIncludeRegions(),
+operation.getExcludeRegions(), (InternalCache) cache);
+
+for (RebalanceOperationPerformer.MemberPRInfo prInfo :
membersForEachRegion) {
+ // Filter out any members using older versions of Geode
+ List viableMembers = filterViableMembers(prInfo);
+
+ if (viableMembers.size() != 0) {
+// Update the MemberPRInfo with the viable members
+prInfo.dsMemberList = viableMembers;
+ } else {
+RestoreRedundancyResultsImpl results = new
RestoreRedundancyResultsImpl();
+
results.setStatusMessage(String.format(NO_MEMBERS_WITH_VERSION_FOR_REGION,
+ADDED_VERSION.getName(), prInfo.region));
+results.setSuccess(false);
+return results;
+ }
+}
+
+List functionResults = new ArrayList<>();
+Object[] functionArgs = new Object[] {operation, checkStatus};
+List completedMembers = new ArrayList<>();
+for (RebalanceOperationPerformer.MemberPRInfo memberPRInfo :
membersForEachRegion) {
+ // Check to see if an earlier function execution has already targeted a
member hosting this
+ // region. If one has, there is no point sending a function for this
region as it has already
+ // had redundancy restored
+ if (!Collections.disjoint(completedMembers, memberPRInfo.dsMemberList)) {
+continue;
+ }
+ // Try the function on the first member for this region
+ DistributedMember targetMember = memberPRInfo.dsMemberList.get(0);
+ RestoreRedundancyResults functionResult =
executeFunctionAndGetFunctionResult(
+ new RestoreRedundancyFunction(), functionArgs, targetMember);
+ if (!functionResult.getSuccess()) {
+// Record the error and
[jira] [Commented] (GEODE-8272) Refactor GFSH Restore Redundancy Command code to allow for REST API
[
https://issues.apache.org/jira/browse/GEODE-8272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138868#comment-17138868
]
ASF GitHub Bot commented on GEODE-8272:
---
DonalEvans commented on a change in pull request #5249:
URL: https://github.com/apache/geode/pull/5249#discussion_r441860267
##
File path:
geode-core/src/test/java/org/apache/geode/management/internal/functions/RestoreRedundancyFunctionTest.java
##
@@ -0,0 +1,163 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express
+ * or implied. See the License for the specific language governing permissions
and limitations under
+ * the License.
+ */
+
+package org.apache.geode.management.internal.functions;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.control.RestoreRedundancyOperation;
+import org.apache.geode.cache.execute.FunctionContext;
+import org.apache.geode.cache.execute.ResultSender;
+import
org.apache.geode.internal.cache.control.SerializableRestoreRedundancyResultsImpl;
+import org.apache.geode.management.operation.RestoreRedundancyRequest;
+import org.apache.geode.management.runtime.RestoreRedundancyResults;
+
+public class RestoreRedundancyFunctionTest {
+ @SuppressWarnings("unchecked")
+ private final FunctionContext mockContext =
mock(FunctionContext.class);
+ private final Cache mockCache = mock(Cache.class, RETURNS_DEEP_STUBS);
+ private final RestoreRedundancyOperation mockOperation =
+ mock(RestoreRedundancyOperation.class, RETURNS_DEEP_STUBS);
+ private final SerializableRestoreRedundancyResultsImpl mockResults =
+ mock(SerializableRestoreRedundancyResultsImpl.class);
+ private final String message = "expected message";
+ private RestoreRedundancyFunction function;
+ private ResultSender resultSender;
+ private ArgumentCaptor
argumentCaptor;
+ private RestoreRedundancyRequest request;
+
+ @Before
+ public void setUp() throws InterruptedException, ExecutionException {
+function = new RestoreRedundancyFunction();
+when(mockContext.getCache()).thenReturn(mockCache);
+request = new RestoreRedundancyRequest();
Review comment:
If we can be reasonably sure that this will remain true, and that no
additional logic will be added to `RestoreRedundancyRequest` in the future,
then this is fine to leave as it is.
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:
us...@infra.apache.org
> Refactor GFSH Restore Redundancy Command code to allow for REST API
> ---
>
> Key: GEODE-8272
> URL: https://issues.apache.org/jira/browse/GEODE-8272
> Project: Geode
> Issue Type: Bug
> Components: gfsh
>Reporter: Mark Hanson
>Assignee: Mark Hanson
>Priority: Major
>
> Refactor GFSH Restore Redundancy Command code to allow for REST API
>
> The core code path is specific to GFSH right now and needs a few changes to
> support the REST API command.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (GEODE-8276) KEYS command returns corrupted keys in some cases
[ https://issues.apache.org/jira/browse/GEODE-8276?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138866#comment-17138866 ] ASF GitHub Bot commented on GEODE-8276: --- dschneider-pivotal opened a new pull request #5272: URL: https://github.com/apache/geode/pull/5272 Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, check Concourse for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to d...@geode.apache.org. 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: us...@infra.apache.org > KEYS command returns corrupted keys in some cases > - > > Key: GEODE-8276 > URL: https://issues.apache.org/jira/browse/GEODE-8276 > Project: Geode > Issue Type: Bug > Components: redis >Reporter: Darrel Schneider >Assignee: Darrel Schneider >Priority: Major > > In some cases the keys returned by the KEYS command will be corrupted. If the > key that is stored contains bytes that when UTF-8 encoded produce multiple > bytes then that key will be corrupted when returned from the KEYS command. > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-8272) Refactor GFSH Restore Redundancy Command code to allow for REST API
[
https://issues.apache.org/jira/browse/GEODE-8272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138864#comment-17138864
]
ASF GitHub Bot commented on GEODE-8272:
---
mhansonp commented on a change in pull request #5249:
URL: https://github.com/apache/geode/pull/5249#discussion_r441858350
##
File path:
geode-core/src/test/java/org/apache/geode/management/internal/functions/RestoreRedundancyFunctionTest.java
##
@@ -0,0 +1,163 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express
+ * or implied. See the License for the specific language governing permissions
and limitations under
+ * the License.
+ */
+
+package org.apache.geode.management.internal.functions;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.control.RestoreRedundancyOperation;
+import org.apache.geode.cache.execute.FunctionContext;
+import org.apache.geode.cache.execute.ResultSender;
+import
org.apache.geode.internal.cache.control.SerializableRestoreRedundancyResultsImpl;
+import org.apache.geode.management.operation.RestoreRedundancyRequest;
+import org.apache.geode.management.runtime.RestoreRedundancyResults;
+
+public class RestoreRedundancyFunctionTest {
+ @SuppressWarnings("unchecked")
+ private final FunctionContext mockContext =
mock(FunctionContext.class);
+ private final Cache mockCache = mock(Cache.class, RETURNS_DEEP_STUBS);
+ private final RestoreRedundancyOperation mockOperation =
+ mock(RestoreRedundancyOperation.class, RETURNS_DEEP_STUBS);
+ private final SerializableRestoreRedundancyResultsImpl mockResults =
+ mock(SerializableRestoreRedundancyResultsImpl.class);
+ private final String message = "expected message";
+ private RestoreRedundancyFunction function;
+ private ResultSender resultSender;
+ private ArgumentCaptor
argumentCaptor;
+ private RestoreRedundancyRequest request;
+
+ @Before
+ public void setUp() throws InterruptedException, ExecutionException {
+function = new RestoreRedundancyFunction();
+when(mockContext.getCache()).thenReturn(mockCache);
+request = new RestoreRedundancyRequest();
Review comment:
I was looking at that and it does seem like its possible, but it doesn't
seem worth it since the request is just a flat data class and doesn't do much
in it.
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:
us...@infra.apache.org
> Refactor GFSH Restore Redundancy Command code to allow for REST API
> ---
>
> Key: GEODE-8272
> URL: https://issues.apache.org/jira/browse/GEODE-8272
> Project: Geode
> Issue Type: Bug
> Components: gfsh
>Reporter: Mark Hanson
>Assignee: Mark Hanson
>Priority: Major
>
> Refactor GFSH Restore Redundancy Command code to allow for REST API
>
> The core code path is specific to GFSH right now and needs a few changes to
> support the REST API command.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Assigned] (GEODE-8276) KEYS command returns corrupted keys in some cases
[ https://issues.apache.org/jira/browse/GEODE-8276?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Darrel Schneider reassigned GEODE-8276: --- Assignee: Darrel Schneider > KEYS command returns corrupted keys in some cases > - > > Key: GEODE-8276 > URL: https://issues.apache.org/jira/browse/GEODE-8276 > Project: Geode > Issue Type: Bug > Components: redis >Reporter: Darrel Schneider >Assignee: Darrel Schneider >Priority: Major > > In some cases the keys returned by the KEYS command will be corrupted. If the > key that is stored contains bytes that when UTF-8 encoded produce multiple > bytes then that key will be corrupted when returned from the KEYS command. > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (GEODE-8276) KEYS command returns corrupted keys in some cases
Darrel Schneider created GEODE-8276: --- Summary: KEYS command returns corrupted keys in some cases Key: GEODE-8276 URL: https://issues.apache.org/jira/browse/GEODE-8276 Project: Geode Issue Type: Bug Components: redis Reporter: Darrel Schneider In some cases the keys returned by the KEYS command will be corrupted. If the key that is stored contains bytes that when UTF-8 encoded produce multiple bytes then that key will be corrupted when returned from the KEYS command. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-6150) Allow use of client/server max-threads with SSL
[ https://issues.apache.org/jira/browse/GEODE-6150?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138849#comment-17138849 ] ASF GitHub Bot commented on GEODE-6150: --- lgtm-com[bot] commented on pull request #5270: URL: https://github.com/apache/geode/pull/5270#issuecomment-645638238 This pull request **introduces 8 alerts** and **fixes 2** when merging 5ee88eb8a6ae6cf2113bfefd8d9d6f17dd6e3df9 into d1e857de0d32742f65768e246c49db88ba90cc47 - [view on LGTM.com](https://lgtm.com/projects/g/apache/geode/rev/pr-0f8ed490f8c9de4d3f88b474712741c005d50a9b) **new alerts:** * 3 for Potential output resource leak * 3 for Dereferenced variable may be null * 2 for Potential input resource leak **fixed alerts:** * 2 for Potential input resource leak 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: us...@infra.apache.org > Allow use of client/server max-threads with SSL > --- > > Key: GEODE-6150 > URL: https://issues.apache.org/jira/browse/GEODE-6150 > Project: Geode > Issue Type: New Feature > Components: client/server >Reporter: Bruce J Schuchardt >Assignee: Mario Ivanac >Priority: Major > Labels: SmallFeature > > Cache servers have a max-threads setting that causes the server to limit the > number of threads used by clients. The implementation uses Java new I/O > though and that doesn't currently play well with SSL/TLS secure > communications. If you attempt to configure the server to use secure > communications _and_ max-threads it throws an IllegalArgumentException with > the message > Selector thread pooling can not be used with client/server SSL. The selector > can be disabled by setting max-threads=0. > The server code should be modified to use the JDK's SSLEngine to implement > SSL/TLS over NIO and get rid of this restriction. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-6150) Allow use of client/server max-threads with SSL
[ https://issues.apache.org/jira/browse/GEODE-6150?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138830#comment-17138830 ] ASF GitHub Bot commented on GEODE-6150: --- mivanac opened a new pull request #5270: URL: https://github.com/apache/geode/pull/5270 WIP: Do not review Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [*] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [*] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [*] Is your initial contribution a single, squashed commit? - [*] Does `gradlew build` run cleanly? - [*] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, check Concourse for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to d...@geode.apache.org. 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: us...@infra.apache.org > Allow use of client/server max-threads with SSL > --- > > Key: GEODE-6150 > URL: https://issues.apache.org/jira/browse/GEODE-6150 > Project: Geode > Issue Type: New Feature > Components: client/server >Reporter: Bruce J Schuchardt >Assignee: Mario Ivanac >Priority: Major > Labels: SmallFeature > > Cache servers have a max-threads setting that causes the server to limit the > number of threads used by clients. The implementation uses Java new I/O > though and that doesn't currently play well with SSL/TLS secure > communications. If you attempt to configure the server to use secure > communications _and_ max-threads it throws an IllegalArgumentException with > the message > Selector thread pooling can not be used with client/server SSL. The selector > can be disabled by setting max-threads=0. > The server code should be modified to use the JDK's SSLEngine to implement > SSL/TLS over NIO and get rid of this restriction. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-8212) Change in PdxType class "<" operator impacts performance
[
https://issues.apache.org/jira/browse/GEODE-8212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138825#comment-17138825
]
ASF GitHub Bot commented on GEODE-8212:
---
alb3rtobr commented on a change in pull request #619:
URL: https://github.com/apache/geode-native/pull/619#discussion_r441827458
##
File path: cppcache/src/PdxTypeRegistry.hpp
##
@@ -53,7 +53,20 @@ typedef std::unordered_map,
dereference_hash>,
dereference_equal_to>>
PreservedHashMap;
-typedef std::map, int32_t, PdxTypeLessThan>
+
+struct PdxTypeHashCode {
+ std::size_t operator()(std::shared_ptr const& pdx) const {
+return pdx ? pdx->hashcode() : 0;
+ }
+};
+
+struct PdxTypeEqualCmp {
+ bool operator()(std::shared_ptr const& first,
std::shared_ptr const& second) const{
+return first->hashcode() == second->hashcode();
Review comment:
Sorry, I read again, I got your point, two objects could generate the
same hash being different :)
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:
us...@infra.apache.org
> Change in PdxType class "<" operator impacts performance
>
>
> Key: GEODE-8212
> URL: https://issues.apache.org/jira/browse/GEODE-8212
> Project: Geode
> Issue Type: Improvement
> Components: native client
>Reporter: Alberto Bustamante Reyes
>Assignee: Alberto Bustamante Reyes
>Priority: Major
> Attachments: performance_after_GEODE-7694.patch
>
>
> While investigating a performance problem of the C++ native client, it has
> been observed that change in the PdxType class "<" operator introduced by
> GEODE-7694 impacts performance of PdxHelper::serializePdx().
> Im attaching a patch with some code and a test case I have used to benchmark
> that method. With the current implementation in develop branch, creating just
> 10 instances is enough to see the difference. The calls to serializePdx take
> these times:
> {noformat}
> Elapsed Time (serializePdx): 32.1322 milliseconds
> Elapsed Time (serializePdx): 3.06044 milliseconds
> Elapsed Time (serializePdx): 1.58791 milliseconds
> Elapsed Time (serializePdx): 1.62466 milliseconds
> Elapsed Time (serializePdx): 1.53676 milliseconds
> Elapsed Time (serializePdx): 1.59278 milliseconds
> Elapsed Time (serializePdx): 1.82878 milliseconds
> Elapsed Time (serializePdx): 1.36811 milliseconds
> Elapsed Time (serializePdx): 1.6956 milliseconds
> Elapsed Time (serializePdx): 1.46873 milliseconds
> Elapsed Time (serializePdx): 1.53206 milliseconds
> {noformat}
> While these are the times using the old "<" operator:
> {noformat}
> Elapsed Time (serializePdx): 56.1524 milliseconds
> Elapsed Time (serializePdx): 0.012327 milliseconds
> Elapsed Time (serializePdx): 0.006325 milliseconds
> Elapsed Time (serializePdx): 0.005419 milliseconds
> Elapsed Time (serializePdx): 0.005266 milliseconds
> Elapsed Time (serializePdx): 0.005662 milliseconds
> Elapsed Time (serializePdx): 0.005407 milliseconds
> Elapsed Time (serializePdx): 0.005286 milliseconds
> Elapsed Time (serializePdx): 0.005467 milliseconds
> Elapsed Time (serializePdx): 0.005276 milliseconds
> Elapsed Time (serializePdx): 0.005298 milliseconds
> {noformat}
> And after creating 50.000 instances, these are the times for the last 10
> calls with the current "<" operator and with the old one respectively::
> {noformat}
> Elapsed Time (serializePdx): 8.11767 milliseconds
> Elapsed Time (serializePdx): 8.15727 milliseconds
> Elapsed Time (serializePdx): 8.09303 milliseconds
> Elapsed Time (serializePdx): 7.89909 milliseconds
> Elapsed Time (serializePdx): 8.37956 milliseconds
> Elapsed Time (serializePdx): 8.21303 milliseconds
> Elapsed Time (serializePdx): 8.14189 milliseconds
> Elapsed Time (serializePdx): 8.21266 milliseconds
> Elapsed Time (serializePdx): 8.21683 milliseconds
> Elapsed Time (serializePdx): 7.78765 milliseconds
> {noformat}
> {noformat}
> Elapsed Time (serializePdx): 0.003014 milliseconds
> Elapsed Time (serializePdx): 0.003 milliseconds
> Elapsed Time (serializePdx): 0.003084 milliseconds
> Elapsed Time (serializePdx): 0.003112 milliseconds
> Elapsed Time (serializePdx): 0.00305 milliseconds
> Elapsed Time (serializePdx): 0.003018 milliseconds
> Elapsed Time (serializePdx): 0.003029 milliseconds
> Elapsed Time (serializePdx): 0.003061 milliseconds
> Elapsed Time (serializePdx): 0.003043 milliseconds
> Elapsed Time (serializePdx): 0.003086 milliseconds
> {noformat}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (GEODE-8241) Locator does not observe locator-wait-time
[
https://issues.apache.org/jira/browse/GEODE-8241?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138819#comment-17138819
]
ASF GitHub Bot commented on GEODE-8241:
---
aaronlindsey commented on pull request #5236:
URL: https://github.com/apache/geode/pull/5236#issuecomment-645616125
I've been running this test in the background while I work and so far it has
passed 300x in a row without failing. I will continue letting it run until it
reaches 1000 iterations.
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:
us...@infra.apache.org
> Locator does not observe locator-wait-time
> --
>
> Key: GEODE-8241
> URL: https://issues.apache.org/jira/browse/GEODE-8241
> Project: Geode
> Issue Type: Bug
>Reporter: Aaron Lindsey
>Assignee: Aaron Lindsey
>Priority: Major
>
> In the case where a locator starts up and is unable to connect to any other
> locators, it may decide to become the membership coordinator even if
> locator-wait-time has not elapsed.
> The following conditional from GMSJoinLeave.java causes the issue. There
> should be an additional check for locator-wait-time before becoming
> coordinator.
> {code:java}
> if (state.joinedMembersContacted <= 0 &&
> (tries >= minimumRetriesBeforeBecomingCoordinator ||
> state.locatorsContacted >= locators.size())) {
> synchronized (viewInstallationLock) {
> becomeCoordinator();
> }
> return true;
> }
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (GEODE-8212) Change in PdxType class "<" operator impacts performance
[
https://issues.apache.org/jira/browse/GEODE-8212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138814#comment-17138814
]
ASF GitHub Bot commented on GEODE-8212:
---
alb3rtobr commented on a change in pull request #619:
URL: https://github.com/apache/geode-native/pull/619#discussion_r441823764
##
File path: cppcache/src/PdxTypeRegistry.hpp
##
@@ -53,7 +53,20 @@ typedef std::unordered_map,
dereference_hash>,
dereference_equal_to>>
PreservedHashMap;
-typedef std::map, int32_t, PdxTypeLessThan>
+
+struct PdxTypeHashCode {
+ std::size_t operator()(std::shared_ptr const& pdx) const {
+return pdx ? pdx->hashcode() : 0;
+ }
+};
+
+struct PdxTypeEqualCmp {
+ bool operator()(std::shared_ptr const& first,
std::shared_ptr const& second) const{
+return first->hashcode() == second->hashcode();
Review comment:
Agree, but I dont want to check if the objects are equals, I just want
to check if two `PdxType` objects have the same hash.
Two `PdxType` objects should be equals if they have the same classname and
their fields have the same types, names and values.
And two keys of the `pdxTypeToTypeId` map will be equals if they have the
same hash, which means they have the same classname, and their fields have the
same types and names (values are not checked here).
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:
us...@infra.apache.org
> Change in PdxType class "<" operator impacts performance
>
>
> Key: GEODE-8212
> URL: https://issues.apache.org/jira/browse/GEODE-8212
> Project: Geode
> Issue Type: Improvement
> Components: native client
>Reporter: Alberto Bustamante Reyes
>Assignee: Alberto Bustamante Reyes
>Priority: Major
> Attachments: performance_after_GEODE-7694.patch
>
>
> While investigating a performance problem of the C++ native client, it has
> been observed that change in the PdxType class "<" operator introduced by
> GEODE-7694 impacts performance of PdxHelper::serializePdx().
> Im attaching a patch with some code and a test case I have used to benchmark
> that method. With the current implementation in develop branch, creating just
> 10 instances is enough to see the difference. The calls to serializePdx take
> these times:
> {noformat}
> Elapsed Time (serializePdx): 32.1322 milliseconds
> Elapsed Time (serializePdx): 3.06044 milliseconds
> Elapsed Time (serializePdx): 1.58791 milliseconds
> Elapsed Time (serializePdx): 1.62466 milliseconds
> Elapsed Time (serializePdx): 1.53676 milliseconds
> Elapsed Time (serializePdx): 1.59278 milliseconds
> Elapsed Time (serializePdx): 1.82878 milliseconds
> Elapsed Time (serializePdx): 1.36811 milliseconds
> Elapsed Time (serializePdx): 1.6956 milliseconds
> Elapsed Time (serializePdx): 1.46873 milliseconds
> Elapsed Time (serializePdx): 1.53206 milliseconds
> {noformat}
> While these are the times using the old "<" operator:
> {noformat}
> Elapsed Time (serializePdx): 56.1524 milliseconds
> Elapsed Time (serializePdx): 0.012327 milliseconds
> Elapsed Time (serializePdx): 0.006325 milliseconds
> Elapsed Time (serializePdx): 0.005419 milliseconds
> Elapsed Time (serializePdx): 0.005266 milliseconds
> Elapsed Time (serializePdx): 0.005662 milliseconds
> Elapsed Time (serializePdx): 0.005407 milliseconds
> Elapsed Time (serializePdx): 0.005286 milliseconds
> Elapsed Time (serializePdx): 0.005467 milliseconds
> Elapsed Time (serializePdx): 0.005276 milliseconds
> Elapsed Time (serializePdx): 0.005298 milliseconds
> {noformat}
> And after creating 50.000 instances, these are the times for the last 10
> calls with the current "<" operator and with the old one respectively::
> {noformat}
> Elapsed Time (serializePdx): 8.11767 milliseconds
> Elapsed Time (serializePdx): 8.15727 milliseconds
> Elapsed Time (serializePdx): 8.09303 milliseconds
> Elapsed Time (serializePdx): 7.89909 milliseconds
> Elapsed Time (serializePdx): 8.37956 milliseconds
> Elapsed Time (serializePdx): 8.21303 milliseconds
> Elapsed Time (serializePdx): 8.14189 milliseconds
> Elapsed Time (serializePdx): 8.21266 milliseconds
> Elapsed Time (serializePdx): 8.21683 milliseconds
> Elapsed Time (serializePdx): 7.78765 milliseconds
> {noformat}
> {noformat}
> Elapsed Time (serializePdx): 0.003014 milliseconds
> Elapsed Time (serializePdx): 0.003 milliseconds
> Elapsed Time (serializePdx): 0.003084 milliseconds
> Elapsed Time (serializePdx): 0.003112 milliseconds
> Elapsed Time (serializePdx):
[jira] [Commented] (GEODE-8241) Locator does not observe locator-wait-time
[
https://issues.apache.org/jira/browse/GEODE-8241?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138813#comment-17138813
]
ASF GitHub Bot commented on GEODE-8241:
---
aaronlindsey commented on a change in pull request #5236:
URL: https://github.com/apache/geode/pull/5236#discussion_r441823437
##
File path:
geode-membership/src/integrationTest/java/org/apache/geode/distributed/internal/membership/gms/MembershipIntegrationTest.java
##
@@ -172,6 +181,92 @@ public void
secondMembershipCanJoinUsingTheSecondLocatorToStart()
stop(locator2, locator1);
}
+ @Test
+ public void secondMembershipPausesForLocatorWaitTime()
+ throws IOException, MemberStartupException {
+
+/*
+ * Start a locator for the coordinator (membership) so we have a port for
it.
+ *
+ * Its locator-wait-time is set to 0 so it eventually (soon after
membership is started) forms a
+ * distributed system and becomes a coordinator.
+ */
+
+final MembershipLocator coordinatorLocator =
createLocator(0);
+coordinatorLocator.start();
+final int coordinatorLocatorPort = coordinatorLocator.getPort();
+
+final Membership coordinatorMembership =
+createMembership(coordinatorLocator, coordinatorLocatorPort);
+
+/*
+ * We have not even started the membership yet — connection attempts will
certainly fail until
+ * we do. This is a bit like the locator (host) not being present in DNS
(yet).
+ */
+
+/*
+ * Start a second locator and membership trying to join via the
coordinator (membership) that
+ * hasn't yet started behind the port.
+ *
+ * Set its locator-wait-time so it'll not become a coordinator right away,
allowing time for the
+ * other member to start and become a coordinator.
+ *
+ * Calculate the locator-wait-time to be greater than the minimum wait
time for connecting to a
+ * locator.
+ */
+
+final MembershipLocator lateJoiningLocator =
createLocator(0);
+lateJoiningLocator.start();
+final int lateJoiningLocatorPort = lateJoiningLocator.getPort();
+
+final int[] lateJoiningMembershipLocatorPorts =
+new int[] {coordinatorLocatorPort, lateJoiningLocatorPort};
+
+final Duration minimumJoinWaitTime = Duration
+.ofMillis(JOIN_RETRY_SLEEP + FIND_LOCATOR_RETRY_SLEEP) // amount of
sleep time per retry
+.multipliedBy(lateJoiningMembershipLocatorPorts.length * 2); //
expected number of retries
+final int locatorWaitTime = (int) (3 * minimumJoinWaitTime.getSeconds());
+
+final MembershipConfig lateJoiningMembershipConfig =
+createMembershipConfig(true, locatorWaitTime,
lateJoiningMembershipLocatorPorts);
+final Membership lateJoiningMembership =
+createMembership(lateJoiningMembershipConfig, lateJoiningLocator);
+
+CompletableFuture lateJoiningMembershipStartup =
executorServiceRule.runAsync(() -> {
+ try {
+start(lateJoiningMembership);
+ } catch (MemberStartupException e) {
+throw new RuntimeException(e);
+ }
+});
+
+/*
+ * Now start the coordinator (membership), after waiting longer than the
minimum wait time for
+ * connecting to a locator but shorter than the locator-wait-time.
+ */
+
+CompletableFuture coordinatorMembershipStartup =
executorServiceRule.runAsync(() -> {
+ try {
+Thread.sleep(2 * minimumJoinWaitTime.toMillis());
Review comment:
I added a comment to the code in the latest commit.
##
File path:
geode-membership/src/integrationTest/java/org/apache/geode/distributed/internal/membership/gms/MembershipIntegrationTest.java
##
@@ -172,6 +181,92 @@ public void
secondMembershipCanJoinUsingTheSecondLocatorToStart()
stop(locator2, locator1);
}
+ @Test
+ public void secondMembershipPausesForLocatorWaitTime()
+ throws IOException, MemberStartupException {
+
+/*
+ * Start a locator for the coordinator (membership) so we have a port for
it.
+ *
+ * Its locator-wait-time is set to 0 so it eventually (soon after
membership is started) forms a
+ * distributed system and becomes a coordinator.
+ */
+
+final MembershipLocator coordinatorLocator =
createLocator(0);
+coordinatorLocator.start();
+final int coordinatorLocatorPort = coordinatorLocator.getPort();
+
+final Membership coordinatorMembership =
+createMembership(coordinatorLocator, coordinatorLocatorPort);
+
+/*
+ * We have not even started the membership yet — connection attempts will
certainly fail until
+ * we do. This is a bit like the locator (host) not being present in DNS
(yet).
+ */
+
+/*
+ * Start a second locator and membership trying to join via the
coordinator (membership) that
+ * hasn't yet started behind the port.
+ *
+ * Set its locator-wait-time so it'll not become a coordinator right
[jira] [Commented] (GEODE-8246) Enforce No Shadow as Error
[ https://issues.apache.org/jira/browse/GEODE-8246?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138803#comment-17138803 ] ASF GitHub Bot commented on GEODE-8246: --- pdxcodemonkey merged pull request #618: URL: https://github.com/apache/geode-native/pull/618 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: us...@infra.apache.org > Enforce No Shadow as Error > -- > > Key: GEODE-8246 > URL: https://issues.apache.org/jira/browse/GEODE-8246 > Project: Geode > Issue Type: Improvement > Components: native client >Reporter: Michael Oleske >Priority: Major > > Given I compile the code without exempting no-shadow > Then it should compile > Note - was marked as a todo -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-8212) Change in PdxType class "<" operator impacts performance
[
https://issues.apache.org/jira/browse/GEODE-8212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138795#comment-17138795
]
ASF GitHub Bot commented on GEODE-8212:
---
pivotal-jbarrett commented on a change in pull request #619:
URL: https://github.com/apache/geode-native/pull/619#discussion_r441813608
##
File path: cppcache/src/PdxType.cpp
##
@@ -557,11 +557,30 @@ bool PdxType::Equals(std::shared_ptr otherObj) {
}
bool PdxType::operator<(const PdxType& other) const {
- auto typeIdLessThan = this->m_geodeTypeId < other.m_geodeTypeId;
- auto typeIdsBothZero =
- (this->m_geodeTypeId == 0) && (other.m_geodeTypeId == 0);
- auto classnameLessThan = this->m_className < other.m_className;
- return (typeIdLessThan || (typeIdsBothZero && classnameLessThan));
+ if (m_geodeTypeId < other.m_geodeTypeId) {
+return true;
+ }
+
+ if ((m_geodeTypeId == 0) && (other.m_geodeTypeId == 0)) {
+return this->m_className < other.m_className;
+ }
+
+ return false;
+}
+
+int32_t PdxType::hashcode() const {
+ std::hash strHash;
Review comment:
See the bottom of cppcache/include/geode/CacheableKey.hpp for how we
export `std::hash` and you can do the same thing for
`std::hash`.
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:
us...@infra.apache.org
> Change in PdxType class "<" operator impacts performance
>
>
> Key: GEODE-8212
> URL: https://issues.apache.org/jira/browse/GEODE-8212
> Project: Geode
> Issue Type: Improvement
> Components: native client
>Reporter: Alberto Bustamante Reyes
>Assignee: Alberto Bustamante Reyes
>Priority: Major
> Attachments: performance_after_GEODE-7694.patch
>
>
> While investigating a performance problem of the C++ native client, it has
> been observed that change in the PdxType class "<" operator introduced by
> GEODE-7694 impacts performance of PdxHelper::serializePdx().
> Im attaching a patch with some code and a test case I have used to benchmark
> that method. With the current implementation in develop branch, creating just
> 10 instances is enough to see the difference. The calls to serializePdx take
> these times:
> {noformat}
> Elapsed Time (serializePdx): 32.1322 milliseconds
> Elapsed Time (serializePdx): 3.06044 milliseconds
> Elapsed Time (serializePdx): 1.58791 milliseconds
> Elapsed Time (serializePdx): 1.62466 milliseconds
> Elapsed Time (serializePdx): 1.53676 milliseconds
> Elapsed Time (serializePdx): 1.59278 milliseconds
> Elapsed Time (serializePdx): 1.82878 milliseconds
> Elapsed Time (serializePdx): 1.36811 milliseconds
> Elapsed Time (serializePdx): 1.6956 milliseconds
> Elapsed Time (serializePdx): 1.46873 milliseconds
> Elapsed Time (serializePdx): 1.53206 milliseconds
> {noformat}
> While these are the times using the old "<" operator:
> {noformat}
> Elapsed Time (serializePdx): 56.1524 milliseconds
> Elapsed Time (serializePdx): 0.012327 milliseconds
> Elapsed Time (serializePdx): 0.006325 milliseconds
> Elapsed Time (serializePdx): 0.005419 milliseconds
> Elapsed Time (serializePdx): 0.005266 milliseconds
> Elapsed Time (serializePdx): 0.005662 milliseconds
> Elapsed Time (serializePdx): 0.005407 milliseconds
> Elapsed Time (serializePdx): 0.005286 milliseconds
> Elapsed Time (serializePdx): 0.005467 milliseconds
> Elapsed Time (serializePdx): 0.005276 milliseconds
> Elapsed Time (serializePdx): 0.005298 milliseconds
> {noformat}
> And after creating 50.000 instances, these are the times for the last 10
> calls with the current "<" operator and with the old one respectively::
> {noformat}
> Elapsed Time (serializePdx): 8.11767 milliseconds
> Elapsed Time (serializePdx): 8.15727 milliseconds
> Elapsed Time (serializePdx): 8.09303 milliseconds
> Elapsed Time (serializePdx): 7.89909 milliseconds
> Elapsed Time (serializePdx): 8.37956 milliseconds
> Elapsed Time (serializePdx): 8.21303 milliseconds
> Elapsed Time (serializePdx): 8.14189 milliseconds
> Elapsed Time (serializePdx): 8.21266 milliseconds
> Elapsed Time (serializePdx): 8.21683 milliseconds
> Elapsed Time (serializePdx): 7.78765 milliseconds
> {noformat}
> {noformat}
> Elapsed Time (serializePdx): 0.003014 milliseconds
> Elapsed Time (serializePdx): 0.003 milliseconds
> Elapsed Time (serializePdx): 0.003084 milliseconds
> Elapsed Time (serializePdx): 0.003112 milliseconds
> Elapsed Time (serializePdx): 0.00305 milliseconds
> Elapsed Time (serializePdx): 0.003018 milliseconds
> Elapsed Time (serializePdx): 0.003029 milliseconds
> Elapsed Time
[jira] [Commented] (GEODE-8212) Change in PdxType class "<" operator impacts performance
[
https://issues.apache.org/jira/browse/GEODE-8212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138791#comment-17138791
]
ASF GitHub Bot commented on GEODE-8212:
---
pdxcodemonkey commented on a change in pull request #619:
URL: https://github.com/apache/geode-native/pull/619#discussion_r441812743
##
File path: cppcache/src/PdxTypeRegistry.hpp
##
@@ -53,7 +53,20 @@ typedef std::unordered_map,
dereference_hash>,
dereference_equal_to>>
PreservedHashMap;
-typedef std::map, int32_t, PdxTypeLessThan>
+
+struct PdxTypeHashCode {
+ std::size_t operator()(std::shared_ptr const& pdx) const {
+return pdx ? pdx->hashcode() : 0;
+ }
+};
+
+struct PdxTypeEqualCmp {
+ bool operator()(std::shared_ptr const& first,
std::shared_ptr const& second) const{
+return first->hashcode() == second->hashcode();
+ }
+};
+
+typedef std::unordered_map, int32_t, PdxTypeHashCode,
PdxTypeEqualCmp>
Review comment:
+1, I like 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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Change in PdxType class "<" operator impacts performance
>
>
> Key: GEODE-8212
> URL: https://issues.apache.org/jira/browse/GEODE-8212
> Project: Geode
> Issue Type: Improvement
> Components: native client
>Reporter: Alberto Bustamante Reyes
>Assignee: Alberto Bustamante Reyes
>Priority: Major
> Attachments: performance_after_GEODE-7694.patch
>
>
> While investigating a performance problem of the C++ native client, it has
> been observed that change in the PdxType class "<" operator introduced by
> GEODE-7694 impacts performance of PdxHelper::serializePdx().
> Im attaching a patch with some code and a test case I have used to benchmark
> that method. With the current implementation in develop branch, creating just
> 10 instances is enough to see the difference. The calls to serializePdx take
> these times:
> {noformat}
> Elapsed Time (serializePdx): 32.1322 milliseconds
> Elapsed Time (serializePdx): 3.06044 milliseconds
> Elapsed Time (serializePdx): 1.58791 milliseconds
> Elapsed Time (serializePdx): 1.62466 milliseconds
> Elapsed Time (serializePdx): 1.53676 milliseconds
> Elapsed Time (serializePdx): 1.59278 milliseconds
> Elapsed Time (serializePdx): 1.82878 milliseconds
> Elapsed Time (serializePdx): 1.36811 milliseconds
> Elapsed Time (serializePdx): 1.6956 milliseconds
> Elapsed Time (serializePdx): 1.46873 milliseconds
> Elapsed Time (serializePdx): 1.53206 milliseconds
> {noformat}
> While these are the times using the old "<" operator:
> {noformat}
> Elapsed Time (serializePdx): 56.1524 milliseconds
> Elapsed Time (serializePdx): 0.012327 milliseconds
> Elapsed Time (serializePdx): 0.006325 milliseconds
> Elapsed Time (serializePdx): 0.005419 milliseconds
> Elapsed Time (serializePdx): 0.005266 milliseconds
> Elapsed Time (serializePdx): 0.005662 milliseconds
> Elapsed Time (serializePdx): 0.005407 milliseconds
> Elapsed Time (serializePdx): 0.005286 milliseconds
> Elapsed Time (serializePdx): 0.005467 milliseconds
> Elapsed Time (serializePdx): 0.005276 milliseconds
> Elapsed Time (serializePdx): 0.005298 milliseconds
> {noformat}
> And after creating 50.000 instances, these are the times for the last 10
> calls with the current "<" operator and with the old one respectively::
> {noformat}
> Elapsed Time (serializePdx): 8.11767 milliseconds
> Elapsed Time (serializePdx): 8.15727 milliseconds
> Elapsed Time (serializePdx): 8.09303 milliseconds
> Elapsed Time (serializePdx): 7.89909 milliseconds
> Elapsed Time (serializePdx): 8.37956 milliseconds
> Elapsed Time (serializePdx): 8.21303 milliseconds
> Elapsed Time (serializePdx): 8.14189 milliseconds
> Elapsed Time (serializePdx): 8.21266 milliseconds
> Elapsed Time (serializePdx): 8.21683 milliseconds
> Elapsed Time (serializePdx): 7.78765 milliseconds
> {noformat}
> {noformat}
> Elapsed Time (serializePdx): 0.003014 milliseconds
> Elapsed Time (serializePdx): 0.003 milliseconds
> Elapsed Time (serializePdx): 0.003084 milliseconds
> Elapsed Time (serializePdx): 0.003112 milliseconds
> Elapsed Time (serializePdx): 0.00305 milliseconds
> Elapsed Time (serializePdx): 0.003018 milliseconds
> Elapsed Time (serializePdx): 0.003029 milliseconds
> Elapsed Time (serializePdx): 0.003061 milliseconds
> Elapsed Time (serializePdx): 0.003043 milliseconds
> Elapsed Time (serializePdx): 0.003086 milliseconds
> {noformat}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (GEODE-8213) C++ native client performance bottleneck in access to serialization registry
[ https://issues.apache.org/jira/browse/GEODE-8213?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138786#comment-17138786 ] ASF GitHub Bot commented on GEODE-8213: --- pivotal-jbarrett commented on pull request #611: URL: https://github.com/apache/geode-native/pull/611#issuecomment-645604306 I think we should close this PR and revisit if after the fixes in https://github.com/apache/geode-native/pull/619 to be merged. After that I think It might just be safe to start by replacing the spinlock with just a plain old mutex to reduce the CPU load. It may not bench as good but solves the issue of CPU load under contention. 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: us...@infra.apache.org > C++ native client performance bottleneck in access to serialization registry > > > Key: GEODE-8213 > URL: https://issues.apache.org/jira/browse/GEODE-8213 > Project: Geode > Issue Type: Improvement > Components: native client >Reporter: Alberto Gomez >Assignee: Alberto Gomez >Priority: Major > Fix For: 1.14.0 > > > It's been observed that when the number of threads used in a Geode client > using PdxSerialization is greater than 8, there is an important drop in > performance. > Analysing the client process behavior with perf, it has been observed a very > high CPU consumption by a spinlock > (apache::geode::util::concurrent::spinlock_mutex::lock) used when accessing > the serialization registry . -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-8212) Change in PdxType class "<" operator impacts performance
[
https://issues.apache.org/jira/browse/GEODE-8212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138782#comment-17138782
]
ASF GitHub Bot commented on GEODE-8212:
---
alb3rtobr commented on a change in pull request #619:
URL: https://github.com/apache/geode-native/pull/619#discussion_r441807945
##
File path: cppcache/src/PdxType.cpp
##
@@ -557,11 +557,30 @@ bool PdxType::Equals(std::shared_ptr otherObj) {
}
bool PdxType::operator<(const PdxType& other) const {
- auto typeIdLessThan = this->m_geodeTypeId < other.m_geodeTypeId;
- auto typeIdsBothZero =
- (this->m_geodeTypeId == 0) && (other.m_geodeTypeId == 0);
- auto classnameLessThan = this->m_className < other.m_className;
- return (typeIdLessThan || (typeIdsBothZero && classnameLessThan));
+ if (m_geodeTypeId < other.m_geodeTypeId) {
+return true;
+ }
+
+ if ((m_geodeTypeId == 0) && (other.m_geodeTypeId == 0)) {
+return this->m_className < other.m_className;
+ }
+
+ return false;
+}
+
+int32_t PdxType::hashcode() const {
+ std::hash strHash;
Review comment:
> Just a thought here. Is it necessary to hash like Java client does in
this case?
No, we just need a hash function to allow identify `PdxType` objects that
will be assigned the same id by the server. We dont need to know the hash
generated by the server.
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:
us...@infra.apache.org
> Change in PdxType class "<" operator impacts performance
>
>
> Key: GEODE-8212
> URL: https://issues.apache.org/jira/browse/GEODE-8212
> Project: Geode
> Issue Type: Improvement
> Components: native client
>Reporter: Alberto Bustamante Reyes
>Assignee: Alberto Bustamante Reyes
>Priority: Major
> Attachments: performance_after_GEODE-7694.patch
>
>
> While investigating a performance problem of the C++ native client, it has
> been observed that change in the PdxType class "<" operator introduced by
> GEODE-7694 impacts performance of PdxHelper::serializePdx().
> Im attaching a patch with some code and a test case I have used to benchmark
> that method. With the current implementation in develop branch, creating just
> 10 instances is enough to see the difference. The calls to serializePdx take
> these times:
> {noformat}
> Elapsed Time (serializePdx): 32.1322 milliseconds
> Elapsed Time (serializePdx): 3.06044 milliseconds
> Elapsed Time (serializePdx): 1.58791 milliseconds
> Elapsed Time (serializePdx): 1.62466 milliseconds
> Elapsed Time (serializePdx): 1.53676 milliseconds
> Elapsed Time (serializePdx): 1.59278 milliseconds
> Elapsed Time (serializePdx): 1.82878 milliseconds
> Elapsed Time (serializePdx): 1.36811 milliseconds
> Elapsed Time (serializePdx): 1.6956 milliseconds
> Elapsed Time (serializePdx): 1.46873 milliseconds
> Elapsed Time (serializePdx): 1.53206 milliseconds
> {noformat}
> While these are the times using the old "<" operator:
> {noformat}
> Elapsed Time (serializePdx): 56.1524 milliseconds
> Elapsed Time (serializePdx): 0.012327 milliseconds
> Elapsed Time (serializePdx): 0.006325 milliseconds
> Elapsed Time (serializePdx): 0.005419 milliseconds
> Elapsed Time (serializePdx): 0.005266 milliseconds
> Elapsed Time (serializePdx): 0.005662 milliseconds
> Elapsed Time (serializePdx): 0.005407 milliseconds
> Elapsed Time (serializePdx): 0.005286 milliseconds
> Elapsed Time (serializePdx): 0.005467 milliseconds
> Elapsed Time (serializePdx): 0.005276 milliseconds
> Elapsed Time (serializePdx): 0.005298 milliseconds
> {noformat}
> And after creating 50.000 instances, these are the times for the last 10
> calls with the current "<" operator and with the old one respectively::
> {noformat}
> Elapsed Time (serializePdx): 8.11767 milliseconds
> Elapsed Time (serializePdx): 8.15727 milliseconds
> Elapsed Time (serializePdx): 8.09303 milliseconds
> Elapsed Time (serializePdx): 7.89909 milliseconds
> Elapsed Time (serializePdx): 8.37956 milliseconds
> Elapsed Time (serializePdx): 8.21303 milliseconds
> Elapsed Time (serializePdx): 8.14189 milliseconds
> Elapsed Time (serializePdx): 8.21266 milliseconds
> Elapsed Time (serializePdx): 8.21683 milliseconds
> Elapsed Time (serializePdx): 7.78765 milliseconds
> {noformat}
> {noformat}
> Elapsed Time (serializePdx): 0.003014 milliseconds
> Elapsed Time (serializePdx): 0.003 milliseconds
> Elapsed Time (serializePdx): 0.003084 milliseconds
> Elapsed Time (serializePdx): 0.003112 milliseconds
> Elapsed Time (serializePdx): 0.00305 milliseconds
> Elapsed
[jira] [Commented] (GEODE-8212) Change in PdxType class "<" operator impacts performance
[
https://issues.apache.org/jira/browse/GEODE-8212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138781#comment-17138781
]
ASF GitHub Bot commented on GEODE-8212:
---
alb3rtobr commented on a change in pull request #619:
URL: https://github.com/apache/geode-native/pull/619#discussion_r441807945
##
File path: cppcache/src/PdxType.cpp
##
@@ -557,11 +557,30 @@ bool PdxType::Equals(std::shared_ptr otherObj) {
}
bool PdxType::operator<(const PdxType& other) const {
- auto typeIdLessThan = this->m_geodeTypeId < other.m_geodeTypeId;
- auto typeIdsBothZero =
- (this->m_geodeTypeId == 0) && (other.m_geodeTypeId == 0);
- auto classnameLessThan = this->m_className < other.m_className;
- return (typeIdLessThan || (typeIdsBothZero && classnameLessThan));
+ if (m_geodeTypeId < other.m_geodeTypeId) {
+return true;
+ }
+
+ if ((m_geodeTypeId == 0) && (other.m_geodeTypeId == 0)) {
+return this->m_className < other.m_className;
+ }
+
+ return false;
+}
+
+int32_t PdxType::hashcode() const {
+ std::hash strHash;
Review comment:
> Just a thought here. Is it necessary to hash like Java client does in
this case?
No, we just need a hash function to allow identify `PdxType` object that
will be assigned the same id by the server. We dont need to know the hash
generated by the server.
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:
us...@infra.apache.org
> Change in PdxType class "<" operator impacts performance
>
>
> Key: GEODE-8212
> URL: https://issues.apache.org/jira/browse/GEODE-8212
> Project: Geode
> Issue Type: Improvement
> Components: native client
>Reporter: Alberto Bustamante Reyes
>Assignee: Alberto Bustamante Reyes
>Priority: Major
> Attachments: performance_after_GEODE-7694.patch
>
>
> While investigating a performance problem of the C++ native client, it has
> been observed that change in the PdxType class "<" operator introduced by
> GEODE-7694 impacts performance of PdxHelper::serializePdx().
> Im attaching a patch with some code and a test case I have used to benchmark
> that method. With the current implementation in develop branch, creating just
> 10 instances is enough to see the difference. The calls to serializePdx take
> these times:
> {noformat}
> Elapsed Time (serializePdx): 32.1322 milliseconds
> Elapsed Time (serializePdx): 3.06044 milliseconds
> Elapsed Time (serializePdx): 1.58791 milliseconds
> Elapsed Time (serializePdx): 1.62466 milliseconds
> Elapsed Time (serializePdx): 1.53676 milliseconds
> Elapsed Time (serializePdx): 1.59278 milliseconds
> Elapsed Time (serializePdx): 1.82878 milliseconds
> Elapsed Time (serializePdx): 1.36811 milliseconds
> Elapsed Time (serializePdx): 1.6956 milliseconds
> Elapsed Time (serializePdx): 1.46873 milliseconds
> Elapsed Time (serializePdx): 1.53206 milliseconds
> {noformat}
> While these are the times using the old "<" operator:
> {noformat}
> Elapsed Time (serializePdx): 56.1524 milliseconds
> Elapsed Time (serializePdx): 0.012327 milliseconds
> Elapsed Time (serializePdx): 0.006325 milliseconds
> Elapsed Time (serializePdx): 0.005419 milliseconds
> Elapsed Time (serializePdx): 0.005266 milliseconds
> Elapsed Time (serializePdx): 0.005662 milliseconds
> Elapsed Time (serializePdx): 0.005407 milliseconds
> Elapsed Time (serializePdx): 0.005286 milliseconds
> Elapsed Time (serializePdx): 0.005467 milliseconds
> Elapsed Time (serializePdx): 0.005276 milliseconds
> Elapsed Time (serializePdx): 0.005298 milliseconds
> {noformat}
> And after creating 50.000 instances, these are the times for the last 10
> calls with the current "<" operator and with the old one respectively::
> {noformat}
> Elapsed Time (serializePdx): 8.11767 milliseconds
> Elapsed Time (serializePdx): 8.15727 milliseconds
> Elapsed Time (serializePdx): 8.09303 milliseconds
> Elapsed Time (serializePdx): 7.89909 milliseconds
> Elapsed Time (serializePdx): 8.37956 milliseconds
> Elapsed Time (serializePdx): 8.21303 milliseconds
> Elapsed Time (serializePdx): 8.14189 milliseconds
> Elapsed Time (serializePdx): 8.21266 milliseconds
> Elapsed Time (serializePdx): 8.21683 milliseconds
> Elapsed Time (serializePdx): 7.78765 milliseconds
> {noformat}
> {noformat}
> Elapsed Time (serializePdx): 0.003014 milliseconds
> Elapsed Time (serializePdx): 0.003 milliseconds
> Elapsed Time (serializePdx): 0.003084 milliseconds
> Elapsed Time (serializePdx): 0.003112 milliseconds
> Elapsed Time (serializePdx): 0.00305 milliseconds
> Elapsed
[jira] [Commented] (GEODE-8212) Change in PdxType class "<" operator impacts performance
[
https://issues.apache.org/jira/browse/GEODE-8212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138780#comment-17138780
]
ASF GitHub Bot commented on GEODE-8212:
---
alb3rtobr commented on a change in pull request #619:
URL: https://github.com/apache/geode-native/pull/619#discussion_r441807945
##
File path: cppcache/src/PdxType.cpp
##
@@ -557,11 +557,30 @@ bool PdxType::Equals(std::shared_ptr otherObj) {
}
bool PdxType::operator<(const PdxType& other) const {
- auto typeIdLessThan = this->m_geodeTypeId < other.m_geodeTypeId;
- auto typeIdsBothZero =
- (this->m_geodeTypeId == 0) && (other.m_geodeTypeId == 0);
- auto classnameLessThan = this->m_className < other.m_className;
- return (typeIdLessThan || (typeIdsBothZero && classnameLessThan));
+ if (m_geodeTypeId < other.m_geodeTypeId) {
+return true;
+ }
+
+ if ((m_geodeTypeId == 0) && (other.m_geodeTypeId == 0)) {
+return this->m_className < other.m_className;
+ }
+
+ return false;
+}
+
+int32_t PdxType::hashcode() const {
+ std::hash strHash;
Review comment:
> Just a thought here. Is it necessary to hash like Java client does in
this case?
No, we just need a hash function to allow identify `PdxType` object that
will be assigned the same id by the server. We dont need to know the hash
generated by the server.
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:
us...@infra.apache.org
> Change in PdxType class "<" operator impacts performance
>
>
> Key: GEODE-8212
> URL: https://issues.apache.org/jira/browse/GEODE-8212
> Project: Geode
> Issue Type: Improvement
> Components: native client
>Reporter: Alberto Bustamante Reyes
>Assignee: Alberto Bustamante Reyes
>Priority: Major
> Attachments: performance_after_GEODE-7694.patch
>
>
> While investigating a performance problem of the C++ native client, it has
> been observed that change in the PdxType class "<" operator introduced by
> GEODE-7694 impacts performance of PdxHelper::serializePdx().
> Im attaching a patch with some code and a test case I have used to benchmark
> that method. With the current implementation in develop branch, creating just
> 10 instances is enough to see the difference. The calls to serializePdx take
> these times:
> {noformat}
> Elapsed Time (serializePdx): 32.1322 milliseconds
> Elapsed Time (serializePdx): 3.06044 milliseconds
> Elapsed Time (serializePdx): 1.58791 milliseconds
> Elapsed Time (serializePdx): 1.62466 milliseconds
> Elapsed Time (serializePdx): 1.53676 milliseconds
> Elapsed Time (serializePdx): 1.59278 milliseconds
> Elapsed Time (serializePdx): 1.82878 milliseconds
> Elapsed Time (serializePdx): 1.36811 milliseconds
> Elapsed Time (serializePdx): 1.6956 milliseconds
> Elapsed Time (serializePdx): 1.46873 milliseconds
> Elapsed Time (serializePdx): 1.53206 milliseconds
> {noformat}
> While these are the times using the old "<" operator:
> {noformat}
> Elapsed Time (serializePdx): 56.1524 milliseconds
> Elapsed Time (serializePdx): 0.012327 milliseconds
> Elapsed Time (serializePdx): 0.006325 milliseconds
> Elapsed Time (serializePdx): 0.005419 milliseconds
> Elapsed Time (serializePdx): 0.005266 milliseconds
> Elapsed Time (serializePdx): 0.005662 milliseconds
> Elapsed Time (serializePdx): 0.005407 milliseconds
> Elapsed Time (serializePdx): 0.005286 milliseconds
> Elapsed Time (serializePdx): 0.005467 milliseconds
> Elapsed Time (serializePdx): 0.005276 milliseconds
> Elapsed Time (serializePdx): 0.005298 milliseconds
> {noformat}
> And after creating 50.000 instances, these are the times for the last 10
> calls with the current "<" operator and with the old one respectively::
> {noformat}
> Elapsed Time (serializePdx): 8.11767 milliseconds
> Elapsed Time (serializePdx): 8.15727 milliseconds
> Elapsed Time (serializePdx): 8.09303 milliseconds
> Elapsed Time (serializePdx): 7.89909 milliseconds
> Elapsed Time (serializePdx): 8.37956 milliseconds
> Elapsed Time (serializePdx): 8.21303 milliseconds
> Elapsed Time (serializePdx): 8.14189 milliseconds
> Elapsed Time (serializePdx): 8.21266 milliseconds
> Elapsed Time (serializePdx): 8.21683 milliseconds
> Elapsed Time (serializePdx): 7.78765 milliseconds
> {noformat}
> {noformat}
> Elapsed Time (serializePdx): 0.003014 milliseconds
> Elapsed Time (serializePdx): 0.003 milliseconds
> Elapsed Time (serializePdx): 0.003084 milliseconds
> Elapsed Time (serializePdx): 0.003112 milliseconds
> Elapsed Time (serializePdx): 0.00305 milliseconds
> Elapsed Time
[jira] [Commented] (GEODE-8212) Change in PdxType class "<" operator impacts performance
[
https://issues.apache.org/jira/browse/GEODE-8212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138777#comment-17138777
]
ASF GitHub Bot commented on GEODE-8212:
---
pivotal-jbarrett commented on a change in pull request #619:
URL: https://github.com/apache/geode-native/pull/619#discussion_r441805042
##
File path: cppcache/src/PdxType.cpp
##
@@ -557,11 +557,30 @@ bool PdxType::Equals(std::shared_ptr otherObj) {
}
bool PdxType::operator<(const PdxType& other) const {
- auto typeIdLessThan = this->m_geodeTypeId < other.m_geodeTypeId;
- auto typeIdsBothZero =
- (this->m_geodeTypeId == 0) && (other.m_geodeTypeId == 0);
- auto classnameLessThan = this->m_className < other.m_className;
- return (typeIdLessThan || (typeIdsBothZero && classnameLessThan));
+ if (m_geodeTypeId < other.m_geodeTypeId) {
+return true;
+ }
+
+ if ((m_geodeTypeId == 0) && (other.m_geodeTypeId == 0)) {
+return this->m_className < other.m_className;
+ }
+
+ return false;
+}
+
+int32_t PdxType::hashcode() const {
+ std::hash strHash;
+ auto result=strHash(this->m_className);
+
+ for (std::vector>::iterator it =
+ m_pdxFieldTypes->begin();
+ it != m_pdxFieldTypes->end(); ++it) {
+auto pdxPtr = *it;
+result = result ^ (strHash(pdxPtr->getClassName()) << 1 );
Review comment:
> > Just to be clear, I didn't mean to suggest we not hash in the
classname at all, just outside the for loop. Also for what it's worth, even
though using classname is still correct, the only instance of PDX we're aware
of that has this problem, i.e. the classname isn't unique, is for JSON
instances, where classname is always "__GEMFIRE_JSON".
>
> oh, I see the confusion: the classname its outside the for loop (line
573). The other classnames you are refering to are the classnames of each
field. If we dont include the name of the field and its classname, the hash
will be the same for an object with a bool called `foo` and for an object with
a string called `foo`.
I don't think the Java implementation will distinguish one OBJECT from
anther OBJECT . I think you just need to has the field name and type and ignore
the class name.
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:
us...@infra.apache.org
> Change in PdxType class "<" operator impacts performance
>
>
> Key: GEODE-8212
> URL: https://issues.apache.org/jira/browse/GEODE-8212
> Project: Geode
> Issue Type: Improvement
> Components: native client
>Reporter: Alberto Bustamante Reyes
>Assignee: Alberto Bustamante Reyes
>Priority: Major
> Attachments: performance_after_GEODE-7694.patch
>
>
> While investigating a performance problem of the C++ native client, it has
> been observed that change in the PdxType class "<" operator introduced by
> GEODE-7694 impacts performance of PdxHelper::serializePdx().
> Im attaching a patch with some code and a test case I have used to benchmark
> that method. With the current implementation in develop branch, creating just
> 10 instances is enough to see the difference. The calls to serializePdx take
> these times:
> {noformat}
> Elapsed Time (serializePdx): 32.1322 milliseconds
> Elapsed Time (serializePdx): 3.06044 milliseconds
> Elapsed Time (serializePdx): 1.58791 milliseconds
> Elapsed Time (serializePdx): 1.62466 milliseconds
> Elapsed Time (serializePdx): 1.53676 milliseconds
> Elapsed Time (serializePdx): 1.59278 milliseconds
> Elapsed Time (serializePdx): 1.82878 milliseconds
> Elapsed Time (serializePdx): 1.36811 milliseconds
> Elapsed Time (serializePdx): 1.6956 milliseconds
> Elapsed Time (serializePdx): 1.46873 milliseconds
> Elapsed Time (serializePdx): 1.53206 milliseconds
> {noformat}
> While these are the times using the old "<" operator:
> {noformat}
> Elapsed Time (serializePdx): 56.1524 milliseconds
> Elapsed Time (serializePdx): 0.012327 milliseconds
> Elapsed Time (serializePdx): 0.006325 milliseconds
> Elapsed Time (serializePdx): 0.005419 milliseconds
> Elapsed Time (serializePdx): 0.005266 milliseconds
> Elapsed Time (serializePdx): 0.005662 milliseconds
> Elapsed Time (serializePdx): 0.005407 milliseconds
> Elapsed Time (serializePdx): 0.005286 milliseconds
> Elapsed Time (serializePdx): 0.005467 milliseconds
> Elapsed Time (serializePdx): 0.005276 milliseconds
> Elapsed Time (serializePdx): 0.005298 milliseconds
> {noformat}
> And after creating 50.000 instances, these are the times for the last 10
> calls with the current "<" operator and with the old one
[jira] [Commented] (GEODE-8212) Change in PdxType class "<" operator impacts performance
[
https://issues.apache.org/jira/browse/GEODE-8212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138768#comment-17138768
]
ASF GitHub Bot commented on GEODE-8212:
---
alb3rtobr commented on a change in pull request #619:
URL: https://github.com/apache/geode-native/pull/619#discussion_r441801421
##
File path: cppcache/src/PdxType.cpp
##
@@ -557,11 +557,30 @@ bool PdxType::Equals(std::shared_ptr otherObj) {
}
bool PdxType::operator<(const PdxType& other) const {
- auto typeIdLessThan = this->m_geodeTypeId < other.m_geodeTypeId;
- auto typeIdsBothZero =
- (this->m_geodeTypeId == 0) && (other.m_geodeTypeId == 0);
- auto classnameLessThan = this->m_className < other.m_className;
- return (typeIdLessThan || (typeIdsBothZero && classnameLessThan));
+ if (m_geodeTypeId < other.m_geodeTypeId) {
+return true;
+ }
+
+ if ((m_geodeTypeId == 0) && (other.m_geodeTypeId == 0)) {
+return this->m_className < other.m_className;
+ }
+
+ return false;
+}
+
+int32_t PdxType::hashcode() const {
+ std::hash strHash;
+ auto result=strHash(this->m_className);
+
+ for (std::vector>::iterator it =
+ m_pdxFieldTypes->begin();
+ it != m_pdxFieldTypes->end(); ++it) {
+auto pdxPtr = *it;
+result = result ^ (strHash(pdxPtr->getClassName()) << 1 );
Review comment:
> It would be good to have a unit test that shows the hashcode doesn't
change if you iterate the fields in a different order...
sure, Im working on that :)
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:
us...@infra.apache.org
> Change in PdxType class "<" operator impacts performance
>
>
> Key: GEODE-8212
> URL: https://issues.apache.org/jira/browse/GEODE-8212
> Project: Geode
> Issue Type: Improvement
> Components: native client
>Reporter: Alberto Bustamante Reyes
>Assignee: Alberto Bustamante Reyes
>Priority: Major
> Attachments: performance_after_GEODE-7694.patch
>
>
> While investigating a performance problem of the C++ native client, it has
> been observed that change in the PdxType class "<" operator introduced by
> GEODE-7694 impacts performance of PdxHelper::serializePdx().
> Im attaching a patch with some code and a test case I have used to benchmark
> that method. With the current implementation in develop branch, creating just
> 10 instances is enough to see the difference. The calls to serializePdx take
> these times:
> {noformat}
> Elapsed Time (serializePdx): 32.1322 milliseconds
> Elapsed Time (serializePdx): 3.06044 milliseconds
> Elapsed Time (serializePdx): 1.58791 milliseconds
> Elapsed Time (serializePdx): 1.62466 milliseconds
> Elapsed Time (serializePdx): 1.53676 milliseconds
> Elapsed Time (serializePdx): 1.59278 milliseconds
> Elapsed Time (serializePdx): 1.82878 milliseconds
> Elapsed Time (serializePdx): 1.36811 milliseconds
> Elapsed Time (serializePdx): 1.6956 milliseconds
> Elapsed Time (serializePdx): 1.46873 milliseconds
> Elapsed Time (serializePdx): 1.53206 milliseconds
> {noformat}
> While these are the times using the old "<" operator:
> {noformat}
> Elapsed Time (serializePdx): 56.1524 milliseconds
> Elapsed Time (serializePdx): 0.012327 milliseconds
> Elapsed Time (serializePdx): 0.006325 milliseconds
> Elapsed Time (serializePdx): 0.005419 milliseconds
> Elapsed Time (serializePdx): 0.005266 milliseconds
> Elapsed Time (serializePdx): 0.005662 milliseconds
> Elapsed Time (serializePdx): 0.005407 milliseconds
> Elapsed Time (serializePdx): 0.005286 milliseconds
> Elapsed Time (serializePdx): 0.005467 milliseconds
> Elapsed Time (serializePdx): 0.005276 milliseconds
> Elapsed Time (serializePdx): 0.005298 milliseconds
> {noformat}
> And after creating 50.000 instances, these are the times for the last 10
> calls with the current "<" operator and with the old one respectively::
> {noformat}
> Elapsed Time (serializePdx): 8.11767 milliseconds
> Elapsed Time (serializePdx): 8.15727 milliseconds
> Elapsed Time (serializePdx): 8.09303 milliseconds
> Elapsed Time (serializePdx): 7.89909 milliseconds
> Elapsed Time (serializePdx): 8.37956 milliseconds
> Elapsed Time (serializePdx): 8.21303 milliseconds
> Elapsed Time (serializePdx): 8.14189 milliseconds
> Elapsed Time (serializePdx): 8.21266 milliseconds
> Elapsed Time (serializePdx): 8.21683 milliseconds
> Elapsed Time (serializePdx): 7.78765 milliseconds
> {noformat}
> {noformat}
> Elapsed Time (serializePdx): 0.003014 milliseconds
> Elapsed Time (serializePdx): 0.003 milliseconds
> Elapsed Time
[jira] [Commented] (GEODE-8212) Change in PdxType class "<" operator impacts performance
[
https://issues.apache.org/jira/browse/GEODE-8212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138769#comment-17138769
]
ASF GitHub Bot commented on GEODE-8212:
---
pivotal-jbarrett commented on a change in pull request #619:
URL: https://github.com/apache/geode-native/pull/619#discussion_r441799345
##
File path: cppcache/src/PdxType.cpp
##
@@ -557,11 +557,30 @@ bool PdxType::Equals(std::shared_ptr otherObj) {
}
bool PdxType::operator<(const PdxType& other) const {
- auto typeIdLessThan = this->m_geodeTypeId < other.m_geodeTypeId;
- auto typeIdsBothZero =
- (this->m_geodeTypeId == 0) && (other.m_geodeTypeId == 0);
- auto classnameLessThan = this->m_className < other.m_className;
- return (typeIdLessThan || (typeIdsBothZero && classnameLessThan));
+ if (m_geodeTypeId < other.m_geodeTypeId) {
+return true;
+ }
+
+ if ((m_geodeTypeId == 0) && (other.m_geodeTypeId == 0)) {
+return this->m_className < other.m_className;
+ }
+
+ return false;
+}
+
+int32_t PdxType::hashcode() const {
+ std::hash strHash;
Review comment:
Just a thought here. Is it necessary to hash like Java client does in
this case? If the `PdxType::hashcode()` is expected to hash similarly to that
of Java version then changes are needed here. If not then I wouldn't try to
implement the `CacheableKey::hashcode` method here and do something more
compatible with C++, like `std::hash` which produces a `size_t`. In fact I
would just create a specialization of `std::hash`.
##
File path: cppcache/src/PdxTypeRegistry.hpp
##
@@ -53,7 +53,20 @@ typedef std::unordered_map,
dereference_hash>,
dereference_equal_to>>
PreservedHashMap;
-typedef std::map, int32_t, PdxTypeLessThan>
+
+struct PdxTypeHashCode {
+ std::size_t operator()(std::shared_ptr const& pdx) const {
+return pdx ? pdx->hashcode() : 0;
+ }
+};
+
+struct PdxTypeEqualCmp {
+ bool operator()(std::shared_ptr const& first,
std::shared_ptr const& second) const{
+return first->hashcode() == second->hashcode();
Review comment:
Equality of hashcode does not guarantee equality of the object. You
should just implement `PdxType::operator==`.
##
File path: cppcache/src/PdxTypeRegistry.hpp
##
@@ -53,7 +53,20 @@ typedef std::unordered_map,
dereference_hash>,
dereference_equal_to>>
PreservedHashMap;
-typedef std::map, int32_t, PdxTypeLessThan>
+
+struct PdxTypeHashCode {
+ std::size_t operator()(std::shared_ptr const& pdx) const {
+return pdx ? pdx->hashcode() : 0;
+ }
+};
+
+struct PdxTypeEqualCmp {
+ bool operator()(std::shared_ptr const& first,
std::shared_ptr const& second) const{
+return first->hashcode() == second->hashcode();
+ }
+};
+
+typedef std::unordered_map, int32_t, PdxTypeHashCode,
PdxTypeEqualCmp>
Review comment:
If you implement `std::hash` and `PdxType::operator==` then you
can use the `dereference_hash>` and
`dereference_equal_to>>` here.
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:
us...@infra.apache.org
> Change in PdxType class "<" operator impacts performance
>
>
> Key: GEODE-8212
> URL: https://issues.apache.org/jira/browse/GEODE-8212
> Project: Geode
> Issue Type: Improvement
> Components: native client
>Reporter: Alberto Bustamante Reyes
>Assignee: Alberto Bustamante Reyes
>Priority: Major
> Attachments: performance_after_GEODE-7694.patch
>
>
> While investigating a performance problem of the C++ native client, it has
> been observed that change in the PdxType class "<" operator introduced by
> GEODE-7694 impacts performance of PdxHelper::serializePdx().
> Im attaching a patch with some code and a test case I have used to benchmark
> that method. With the current implementation in develop branch, creating just
> 10 instances is enough to see the difference. The calls to serializePdx take
> these times:
> {noformat}
> Elapsed Time (serializePdx): 32.1322 milliseconds
> Elapsed Time (serializePdx): 3.06044 milliseconds
> Elapsed Time (serializePdx): 1.58791 milliseconds
> Elapsed Time (serializePdx): 1.62466 milliseconds
> Elapsed Time (serializePdx): 1.53676 milliseconds
> Elapsed Time (serializePdx): 1.59278 milliseconds
> Elapsed Time (serializePdx): 1.82878 milliseconds
> Elapsed Time (serializePdx): 1.36811 milliseconds
> Elapsed Time (serializePdx): 1.6956 milliseconds
> Elapsed Time (serializePdx): 1.46873 milliseconds
>
[jira] [Commented] (GEODE-8149) Introduce new parameter to control the feature
[ https://issues.apache.org/jira/browse/GEODE-8149?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138770#comment-17138770 ] ASF GitHub Bot commented on GEODE-8149: --- davebarnes97 commented on pull request #5139: URL: https://github.com/apache/geode/pull/5139#issuecomment-645594536 > OK, I will create sub-ticket to handle User Guide docs. I'm not finding the sub-ticket for docs. Please provide the ticket number - 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Introduce new parameter to control the feature > -- > > Key: GEODE-8149 > URL: https://issues.apache.org/jira/browse/GEODE-8149 > Project: Geode > Issue Type: Sub-task >Reporter: Jakov Varenina >Assignee: Jakov Varenina >Priority: Major > > * New parameter _security-cn-auth-enabled_ (default value "false") parameter > should be introduced to control this new feature. It should be allowed to set > only if mutual SSL is enabled on all components: _ssl- > ssl-require-authentication == true && ssl-web-require-authentication == true > && ssl-enabled-components==(ALL or all components listed) && > security-manager.isSet == true_ > * New property _security-common-name_ for _security-manager.authentication_ > method should be introduced > * New integration and unit tests -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-8212) Change in PdxType class "<" operator impacts performance
[
https://issues.apache.org/jira/browse/GEODE-8212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138767#comment-17138767
]
ASF GitHub Bot commented on GEODE-8212:
---
alb3rtobr commented on a change in pull request #619:
URL: https://github.com/apache/geode-native/pull/619#discussion_r441800944
##
File path: cppcache/src/PdxType.cpp
##
@@ -557,11 +557,30 @@ bool PdxType::Equals(std::shared_ptr otherObj) {
}
bool PdxType::operator<(const PdxType& other) const {
- auto typeIdLessThan = this->m_geodeTypeId < other.m_geodeTypeId;
- auto typeIdsBothZero =
- (this->m_geodeTypeId == 0) && (other.m_geodeTypeId == 0);
- auto classnameLessThan = this->m_className < other.m_className;
- return (typeIdLessThan || (typeIdsBothZero && classnameLessThan));
+ if (m_geodeTypeId < other.m_geodeTypeId) {
+return true;
+ }
+
+ if ((m_geodeTypeId == 0) && (other.m_geodeTypeId == 0)) {
+return this->m_className < other.m_className;
+ }
+
+ return false;
+}
+
+int32_t PdxType::hashcode() const {
+ std::hash strHash;
+ auto result=strHash(this->m_className);
+
+ for (std::vector>::iterator it =
+ m_pdxFieldTypes->begin();
+ it != m_pdxFieldTypes->end(); ++it) {
+auto pdxPtr = *it;
+result = result ^ (strHash(pdxPtr->getClassName()) << 1 );
Review comment:
> Just to be clear, I didn't mean to suggest we not hash in the
classname at all, just outside the for loop. Also for what it's worth, even
though using classname is still correct, the only instance of PDX we're aware
of that has this problem, i.e. the classname isn't unique, is for JSON
instances, where classname is always "__GEMFIRE_JSON".
oh, I see the confusion: the classname its outside the for loop (line 573).
The other classnames you are refering to are the classnames of each field. If
we dont include the name of the field and its classname, the hash will be the
same for an object with a bool called `foo` and for an object with a string
called `foo`.
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:
us...@infra.apache.org
> Change in PdxType class "<" operator impacts performance
>
>
> Key: GEODE-8212
> URL: https://issues.apache.org/jira/browse/GEODE-8212
> Project: Geode
> Issue Type: Improvement
> Components: native client
>Reporter: Alberto Bustamante Reyes
>Assignee: Alberto Bustamante Reyes
>Priority: Major
> Attachments: performance_after_GEODE-7694.patch
>
>
> While investigating a performance problem of the C++ native client, it has
> been observed that change in the PdxType class "<" operator introduced by
> GEODE-7694 impacts performance of PdxHelper::serializePdx().
> Im attaching a patch with some code and a test case I have used to benchmark
> that method. With the current implementation in develop branch, creating just
> 10 instances is enough to see the difference. The calls to serializePdx take
> these times:
> {noformat}
> Elapsed Time (serializePdx): 32.1322 milliseconds
> Elapsed Time (serializePdx): 3.06044 milliseconds
> Elapsed Time (serializePdx): 1.58791 milliseconds
> Elapsed Time (serializePdx): 1.62466 milliseconds
> Elapsed Time (serializePdx): 1.53676 milliseconds
> Elapsed Time (serializePdx): 1.59278 milliseconds
> Elapsed Time (serializePdx): 1.82878 milliseconds
> Elapsed Time (serializePdx): 1.36811 milliseconds
> Elapsed Time (serializePdx): 1.6956 milliseconds
> Elapsed Time (serializePdx): 1.46873 milliseconds
> Elapsed Time (serializePdx): 1.53206 milliseconds
> {noformat}
> While these are the times using the old "<" operator:
> {noformat}
> Elapsed Time (serializePdx): 56.1524 milliseconds
> Elapsed Time (serializePdx): 0.012327 milliseconds
> Elapsed Time (serializePdx): 0.006325 milliseconds
> Elapsed Time (serializePdx): 0.005419 milliseconds
> Elapsed Time (serializePdx): 0.005266 milliseconds
> Elapsed Time (serializePdx): 0.005662 milliseconds
> Elapsed Time (serializePdx): 0.005407 milliseconds
> Elapsed Time (serializePdx): 0.005286 milliseconds
> Elapsed Time (serializePdx): 0.005467 milliseconds
> Elapsed Time (serializePdx): 0.005276 milliseconds
> Elapsed Time (serializePdx): 0.005298 milliseconds
> {noformat}
> And after creating 50.000 instances, these are the times for the last 10
> calls with the current "<" operator and with the old one respectively::
> {noformat}
> Elapsed Time (serializePdx): 8.11767 milliseconds
> Elapsed Time (serializePdx): 8.15727 milliseconds
> Elapsed Time (serializePdx): 8.09303 milliseconds
>
[jira] [Commented] (GEODE-8250) Improve documentation for logging
[ https://issues.apache.org/jira/browse/GEODE-8250?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138761#comment-17138761 ] ASF GitHub Bot commented on GEODE-8250: --- kirklund opened a new pull request #5268: URL: https://github.com/apache/geode/pull/5268 New tests to discover and verify that use of custom log configs works when using GFSH to start Locator and Server. 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: us...@infra.apache.org > Improve documentation for logging > - > > Key: GEODE-8250 > URL: https://issues.apache.org/jira/browse/GEODE-8250 > Project: Geode > Issue Type: Improvement > Components: docs >Reporter: Kirk Lund >Assignee: Kirk Lund >Priority: Major > Labels: GeodeOperationAPI, docs, logging > > The documentation pages for logging need some fixes, clarification, and > examples. Specifically the docs need updating to cover the newer geode-log4j > module and its custom appenders. Examples should include how to use custom > logging config with optional usage of the latest version of Geode's custom > appenders and pattern converters. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-8273) Cleanup GfshExecution and GfshScript classes in geode-junit
[ https://issues.apache.org/jira/browse/GEODE-8273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138759#comment-17138759 ] ASF GitHub Bot commented on GEODE-8273: --- kirklund opened a new pull request #5267: URL: https://github.com/apache/geode/pull/5267 * Use GeodeAwaitility timeout instead of hardcoded timeouts. * Use just one source for Charset. * Minor formatting changes. 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: us...@infra.apache.org > Cleanup GfshExecution and GfshScript classes in geode-junit > --- > > Key: GEODE-8273 > URL: https://issues.apache.org/jira/browse/GEODE-8273 > Project: Geode > Issue Type: Wish > Components: tests >Reporter: Kirk Lund >Assignee: Kirk Lund >Priority: Major > > Cleanup GfshExecution and GfshScript classes in geode-junit. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-8274) Improve readability of Version comparisons
[
https://issues.apache.org/jira/browse/GEODE-8274?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138752#comment-17138752
]
ASF GitHub Bot commented on GEODE-8274:
---
pivotal-jbarrett opened a new pull request #5266:
URL: https://github.com/apache/geode/pull/5266
Thank you for submitting a contribution to Apache Geode.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in
the commit message?
- [ ] Has your PR been rebased against the latest commit within the target
branch (typically `develop`)?
- [ ] Is your initial contribution a single, squashed commit?
- [ ] Does `gradlew build` run cleanly?
- [ ] Have you written or updated unit tests to verify your changes?
- [ ] If adding new dependencies to the code, are these dependencies
licensed in a way that is compatible for inclusion under [ASF
2.0](http://www.apache.org/legal/resolved.html#category-a)?
### Note:
Please ensure that once the PR is submitted, check Concourse for build
issues and
submit an update to your PR as soon as possible. If you need help, please
send an
email to d...@geode.apache.org.
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:
us...@infra.apache.org
> Improve readability of Version comparisons
> --
>
> Key: GEODE-8274
> URL: https://issues.apache.org/jira/browse/GEODE-8274
> Project: Geode
> Issue Type: Improvement
> Components: serialization
>Reporter: Jacob Barrett
>Assignee: Jacob Barrett
>Priority: Major
>
> Currently Version comparisons are done with {{Version.compareTo(Version) <=>
> 0}} operations. It is difficult to reason with some of the combinations to
> understand if we are looking for newer or older versions. Convert to some
> natural language style like {{Version.isNewerThan(Version)}}.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Created] (GEODE-8274) Improve readability of Version comparisons
Jacob Barrett created GEODE-8274:
Summary: Improve readability of Version comparisons
Key: GEODE-8274
URL: https://issues.apache.org/jira/browse/GEODE-8274
Project: Geode
Issue Type: Improvement
Components: serialization
Reporter: Jacob Barrett
Currently Version comparisons are done with {{Version.compareTo(Version) <=>
0}} operations. It is difficult to reason with some of the combinations to
understand if we are looking for newer or older versions. Convert to some
natural language style like {{Version.isNewerThan(Version)}}.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Assigned] (GEODE-8274) Improve readability of Version comparisons
[
https://issues.apache.org/jira/browse/GEODE-8274?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jacob Barrett reassigned GEODE-8274:
Assignee: Jacob Barrett
> Improve readability of Version comparisons
> --
>
> Key: GEODE-8274
> URL: https://issues.apache.org/jira/browse/GEODE-8274
> Project: Geode
> Issue Type: Improvement
> Components: serialization
>Reporter: Jacob Barrett
>Assignee: Jacob Barrett
>Priority: Major
>
> Currently Version comparisons are done with {{Version.compareTo(Version) <=>
> 0}} operations. It is difficult to reason with some of the combinations to
> understand if we are looking for newer or older versions. Convert to some
> natural language style like {{Version.isNewerThan(Version)}}.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Assigned] (GEODE-8273) Cleanup GfshExecution and GfshScript classes in geode-junit
[ https://issues.apache.org/jira/browse/GEODE-8273?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Kirk Lund reassigned GEODE-8273: Assignee: Kirk Lund > Cleanup GfshExecution and GfshScript classes in geode-junit > --- > > Key: GEODE-8273 > URL: https://issues.apache.org/jira/browse/GEODE-8273 > Project: Geode > Issue Type: Wish > Components: tests >Reporter: Kirk Lund >Assignee: Kirk Lund >Priority: Major > > Cleanup GfshExecution and GfshScript classes in geode-junit. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (GEODE-8273) Cleanup GfshExecution and GfshScript classes in geode-junit
Kirk Lund created GEODE-8273: Summary: Cleanup GfshExecution and GfshScript classes in geode-junit Key: GEODE-8273 URL: https://issues.apache.org/jira/browse/GEODE-8273 Project: Geode Issue Type: Wish Components: tests Reporter: Kirk Lund Cleanup GfshExecution and GfshScript classes in geode-junit. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-8270) Reorganize Redis API for Geode test packages to match main package structure
[ https://issues.apache.org/jira/browse/GEODE-8270?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138734#comment-17138734 ] ASF subversion and git services commented on GEODE-8270: Commit d1e857de0d32742f65768e246c49db88ba90cc47 in geode's branch refs/heads/develop from Sarah Abbey [ https://gitbox.apache.org/repos/asf?p=geode.git;h=d1e857d ] GEODE-8270: Reorganize test packages to match main package structure (#5263) > Reorganize Redis API for Geode test packages to match main package structure > > > Key: GEODE-8270 > URL: https://issues.apache.org/jira/browse/GEODE-8270 > Project: Geode > Issue Type: Improvement > Components: redis >Reporter: Sarah Abbey >Priority: Major > Fix For: 1.14.0 > > > Reorganize test packages to match main package structure so it's easier to > find relevant tests -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Resolved] (GEODE-8270) Reorganize Redis API for Geode test packages to match main package structure
[ https://issues.apache.org/jira/browse/GEODE-8270?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Darrel Schneider resolved GEODE-8270. - Fix Version/s: 1.14.0 Resolution: Fixed > Reorganize Redis API for Geode test packages to match main package structure > > > Key: GEODE-8270 > URL: https://issues.apache.org/jira/browse/GEODE-8270 > Project: Geode > Issue Type: Improvement > Components: redis >Reporter: Sarah Abbey >Priority: Major > Fix For: 1.14.0 > > > Reorganize test packages to match main package structure so it's easier to > find relevant tests -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-8270) Reorganize Redis API for Geode test packages to match main package structure
[ https://issues.apache.org/jira/browse/GEODE-8270?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138733#comment-17138733 ] ASF GitHub Bot commented on GEODE-8270: --- dschneider-pivotal merged pull request #5263: URL: https://github.com/apache/geode/pull/5263 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: us...@infra.apache.org > Reorganize Redis API for Geode test packages to match main package structure > > > Key: GEODE-8270 > URL: https://issues.apache.org/jira/browse/GEODE-8270 > Project: Geode > Issue Type: Improvement > Components: redis >Reporter: Sarah Abbey >Priority: Major > > Reorganize test packages to match main package structure so it's easier to > find relevant tests -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-8212) Change in PdxType class "<" operator impacts performance
[
https://issues.apache.org/jira/browse/GEODE-8212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138716#comment-17138716
]
ASF GitHub Bot commented on GEODE-8212:
---
pdxcodemonkey commented on a change in pull request #619:
URL: https://github.com/apache/geode-native/pull/619#discussion_r441751119
##
File path: cppcache/src/PdxType.cpp
##
@@ -557,11 +557,30 @@ bool PdxType::Equals(std::shared_ptr otherObj) {
}
bool PdxType::operator<(const PdxType& other) const {
- auto typeIdLessThan = this->m_geodeTypeId < other.m_geodeTypeId;
- auto typeIdsBothZero =
- (this->m_geodeTypeId == 0) && (other.m_geodeTypeId == 0);
- auto classnameLessThan = this->m_className < other.m_className;
- return (typeIdLessThan || (typeIdsBothZero && classnameLessThan));
+ if (m_geodeTypeId < other.m_geodeTypeId) {
+return true;
+ }
+
+ if ((m_geodeTypeId == 0) && (other.m_geodeTypeId == 0)) {
+return this->m_className < other.m_className;
+ }
+
+ return false;
+}
+
+int32_t PdxType::hashcode() const {
+ std::hash strHash;
+ auto result=strHash(this->m_className);
+
+ for (std::vector>::iterator it =
+ m_pdxFieldTypes->begin();
+ it != m_pdxFieldTypes->end(); ++it) {
+auto pdxPtr = *it;
+result = result ^ (strHash(pdxPtr->getClassName()) << 1 );
Review comment:
It would be good to have a unit test that shows the hashcode doesn't
change if you iterate the fields in a different order...
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:
us...@infra.apache.org
> Change in PdxType class "<" operator impacts performance
>
>
> Key: GEODE-8212
> URL: https://issues.apache.org/jira/browse/GEODE-8212
> Project: Geode
> Issue Type: Improvement
> Components: native client
>Reporter: Alberto Bustamante Reyes
>Assignee: Alberto Bustamante Reyes
>Priority: Major
> Attachments: performance_after_GEODE-7694.patch
>
>
> While investigating a performance problem of the C++ native client, it has
> been observed that change in the PdxType class "<" operator introduced by
> GEODE-7694 impacts performance of PdxHelper::serializePdx().
> Im attaching a patch with some code and a test case I have used to benchmark
> that method. With the current implementation in develop branch, creating just
> 10 instances is enough to see the difference. The calls to serializePdx take
> these times:
> {noformat}
> Elapsed Time (serializePdx): 32.1322 milliseconds
> Elapsed Time (serializePdx): 3.06044 milliseconds
> Elapsed Time (serializePdx): 1.58791 milliseconds
> Elapsed Time (serializePdx): 1.62466 milliseconds
> Elapsed Time (serializePdx): 1.53676 milliseconds
> Elapsed Time (serializePdx): 1.59278 milliseconds
> Elapsed Time (serializePdx): 1.82878 milliseconds
> Elapsed Time (serializePdx): 1.36811 milliseconds
> Elapsed Time (serializePdx): 1.6956 milliseconds
> Elapsed Time (serializePdx): 1.46873 milliseconds
> Elapsed Time (serializePdx): 1.53206 milliseconds
> {noformat}
> While these are the times using the old "<" operator:
> {noformat}
> Elapsed Time (serializePdx): 56.1524 milliseconds
> Elapsed Time (serializePdx): 0.012327 milliseconds
> Elapsed Time (serializePdx): 0.006325 milliseconds
> Elapsed Time (serializePdx): 0.005419 milliseconds
> Elapsed Time (serializePdx): 0.005266 milliseconds
> Elapsed Time (serializePdx): 0.005662 milliseconds
> Elapsed Time (serializePdx): 0.005407 milliseconds
> Elapsed Time (serializePdx): 0.005286 milliseconds
> Elapsed Time (serializePdx): 0.005467 milliseconds
> Elapsed Time (serializePdx): 0.005276 milliseconds
> Elapsed Time (serializePdx): 0.005298 milliseconds
> {noformat}
> And after creating 50.000 instances, these are the times for the last 10
> calls with the current "<" operator and with the old one respectively::
> {noformat}
> Elapsed Time (serializePdx): 8.11767 milliseconds
> Elapsed Time (serializePdx): 8.15727 milliseconds
> Elapsed Time (serializePdx): 8.09303 milliseconds
> Elapsed Time (serializePdx): 7.89909 milliseconds
> Elapsed Time (serializePdx): 8.37956 milliseconds
> Elapsed Time (serializePdx): 8.21303 milliseconds
> Elapsed Time (serializePdx): 8.14189 milliseconds
> Elapsed Time (serializePdx): 8.21266 milliseconds
> Elapsed Time (serializePdx): 8.21683 milliseconds
> Elapsed Time (serializePdx): 7.78765 milliseconds
> {noformat}
> {noformat}
> Elapsed Time (serializePdx): 0.003014 milliseconds
> Elapsed Time (serializePdx): 0.003 milliseconds
> Elapsed Time (serializePdx): 0.003084 milliseconds
>
[jira] [Commented] (GEODE-8241) Locator does not observe locator-wait-time
[
https://issues.apache.org/jira/browse/GEODE-8241?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138713#comment-17138713
]
ASF GitHub Bot commented on GEODE-8241:
---
aaronlindsey commented on pull request #5236:
URL: https://github.com/apache/geode/pull/5236#issuecomment-645549135
@Bill what do you think of mocking out calls to `System.currentTimeMillis()`
in `GMSJoinLeave`? Allowing the test to control the clock might allow a more
deterministic test. I'm not sure if it would work, though, because there are a
lot of usages of `System.currentTimeMillis()` in `GMSJoinLeave` that our test
doesn't care about which would also be affected.
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:
us...@infra.apache.org
> Locator does not observe locator-wait-time
> --
>
> Key: GEODE-8241
> URL: https://issues.apache.org/jira/browse/GEODE-8241
> Project: Geode
> Issue Type: Bug
>Reporter: Aaron Lindsey
>Assignee: Aaron Lindsey
>Priority: Major
>
> In the case where a locator starts up and is unable to connect to any other
> locators, it may decide to become the membership coordinator even if
> locator-wait-time has not elapsed.
> The following conditional from GMSJoinLeave.java causes the issue. There
> should be an additional check for locator-wait-time before becoming
> coordinator.
> {code:java}
> if (state.joinedMembersContacted <= 0 &&
> (tries >= minimumRetriesBeforeBecomingCoordinator ||
> state.locatorsContacted >= locators.size())) {
> synchronized (viewInstallationLock) {
> becomeCoordinator();
> }
> return true;
> }
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (GEODE-8212) Change in PdxType class "<" operator impacts performance
[
https://issues.apache.org/jira/browse/GEODE-8212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138714#comment-17138714
]
ASF GitHub Bot commented on GEODE-8212:
---
pdxcodemonkey commented on a change in pull request #619:
URL: https://github.com/apache/geode-native/pull/619#discussion_r441749254
##
File path: cppcache/src/PdxType.cpp
##
@@ -557,11 +557,30 @@ bool PdxType::Equals(std::shared_ptr otherObj) {
}
bool PdxType::operator<(const PdxType& other) const {
- auto typeIdLessThan = this->m_geodeTypeId < other.m_geodeTypeId;
- auto typeIdsBothZero =
- (this->m_geodeTypeId == 0) && (other.m_geodeTypeId == 0);
- auto classnameLessThan = this->m_className < other.m_className;
- return (typeIdLessThan || (typeIdsBothZero && classnameLessThan));
+ if (m_geodeTypeId < other.m_geodeTypeId) {
+return true;
+ }
+
+ if ((m_geodeTypeId == 0) && (other.m_geodeTypeId == 0)) {
+return this->m_className < other.m_className;
+ }
+
+ return false;
+}
+
+int32_t PdxType::hashcode() const {
+ std::hash strHash;
+ auto result=strHash(this->m_className);
+
+ for (std::vector>::iterator it =
+ m_pdxFieldTypes->begin();
+ it != m_pdxFieldTypes->end(); ++it) {
+auto pdxPtr = *it;
+result = result ^ (strHash(pdxPtr->getClassName()) << 1 );
Review comment:
Just to be clear, I didn't mean to suggest we not hash in the classname
at all, just outside the for loop. Also for what it's worth, even though using
classname is still correct, the only instance of PDX we're aware of that has
this problem, i.e. the classname isn't unique, is for JSON instances, where
classname is always "__GEMFIRE_JSON".
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:
us...@infra.apache.org
> Change in PdxType class "<" operator impacts performance
>
>
> Key: GEODE-8212
> URL: https://issues.apache.org/jira/browse/GEODE-8212
> Project: Geode
> Issue Type: Improvement
> Components: native client
>Reporter: Alberto Bustamante Reyes
>Assignee: Alberto Bustamante Reyes
>Priority: Major
> Attachments: performance_after_GEODE-7694.patch
>
>
> While investigating a performance problem of the C++ native client, it has
> been observed that change in the PdxType class "<" operator introduced by
> GEODE-7694 impacts performance of PdxHelper::serializePdx().
> Im attaching a patch with some code and a test case I have used to benchmark
> that method. With the current implementation in develop branch, creating just
> 10 instances is enough to see the difference. The calls to serializePdx take
> these times:
> {noformat}
> Elapsed Time (serializePdx): 32.1322 milliseconds
> Elapsed Time (serializePdx): 3.06044 milliseconds
> Elapsed Time (serializePdx): 1.58791 milliseconds
> Elapsed Time (serializePdx): 1.62466 milliseconds
> Elapsed Time (serializePdx): 1.53676 milliseconds
> Elapsed Time (serializePdx): 1.59278 milliseconds
> Elapsed Time (serializePdx): 1.82878 milliseconds
> Elapsed Time (serializePdx): 1.36811 milliseconds
> Elapsed Time (serializePdx): 1.6956 milliseconds
> Elapsed Time (serializePdx): 1.46873 milliseconds
> Elapsed Time (serializePdx): 1.53206 milliseconds
> {noformat}
> While these are the times using the old "<" operator:
> {noformat}
> Elapsed Time (serializePdx): 56.1524 milliseconds
> Elapsed Time (serializePdx): 0.012327 milliseconds
> Elapsed Time (serializePdx): 0.006325 milliseconds
> Elapsed Time (serializePdx): 0.005419 milliseconds
> Elapsed Time (serializePdx): 0.005266 milliseconds
> Elapsed Time (serializePdx): 0.005662 milliseconds
> Elapsed Time (serializePdx): 0.005407 milliseconds
> Elapsed Time (serializePdx): 0.005286 milliseconds
> Elapsed Time (serializePdx): 0.005467 milliseconds
> Elapsed Time (serializePdx): 0.005276 milliseconds
> Elapsed Time (serializePdx): 0.005298 milliseconds
> {noformat}
> And after creating 50.000 instances, these are the times for the last 10
> calls with the current "<" operator and with the old one respectively::
> {noformat}
> Elapsed Time (serializePdx): 8.11767 milliseconds
> Elapsed Time (serializePdx): 8.15727 milliseconds
> Elapsed Time (serializePdx): 8.09303 milliseconds
> Elapsed Time (serializePdx): 7.89909 milliseconds
> Elapsed Time (serializePdx): 8.37956 milliseconds
> Elapsed Time (serializePdx): 8.21303 milliseconds
> Elapsed Time (serializePdx): 8.14189 milliseconds
> Elapsed Time (serializePdx): 8.21266 milliseconds
> Elapsed Time (serializePdx): 8.21683 milliseconds
> Elapsed Time
[jira] [Commented] (GEODE-8251) exception creating domain.Configuration stops locator startup after rolling upgrade
[
https://issues.apache.org/jira/browse/GEODE-8251?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138688#comment-17138688
]
ASF GitHub Bot commented on GEODE-8251:
---
albertogpz commented on pull request #5257:
URL: https://github.com/apache/geode/pull/5257#issuecomment-645524694
> > > > What's the point for these changes?
> > > > I have ran the test case without them and it passes.
> > > > Also, I have searched in the Geode code for the use of this file and
have only found it in `ObjectInputStreamFilterWrapper.java`. From what I see
there, only the first part of every line, the class name, is used.
> > > > What am I missing?
> > >
> > >
> > > the previous version of Deployment has variable name "jarFileName"
instead of "fileName", when we are deserializing a previous version bytes, if
the variable name doesn't match, it won't take the value. We should have a test
to verify that, but I saw the behavior in the debugger.
> >
> >
> > I understand that. What I do not get are the changes in the
> > > > What's the point for these changes?
> > > > I have ran the test case without them and it passes.
> > > > Also, I have searched in the Geode code for the use of this file and
have only found it in `ObjectInputStreamFilterWrapper.java`. From what I see
there, only the first part of every line, the class name, is used.
> > > > What am I missing?
> > >
> > >
> > > the previous version of Deployment has variable name "jarFileName"
instead of "fileName", when we are deserializing a previous version bytes, if
the variable name doesn't match, it won't take the value. We should have a test
to verify that, but I saw the behavior in the debugger.
> >
> >
> > I understand the change in the members of the class. What I do not get
is why it needs to be reflected in
sanctioned-geode-management-serializables.txt.
> > Who is accessing that file?
>
> the `AnalyzeManagementSerializablesJUnitTest` is making sure that we don't
make random changes in those serialized classes. Every change in the sanction
list poses a potential upgrade issue. So the fact that the test is broken will
make the developer think twice about the changes he/she is making to the class.
I see now. Thanks for the clarification.
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:
us...@infra.apache.org
> exception creating domain.Configuration stops locator startup after rolling
> upgrade
> ---
>
> Key: GEODE-8251
> URL: https://issues.apache.org/jira/browse/GEODE-8251
> Project: Geode
> Issue Type: Bug
> Components: configuration
>Affects Versions: 1.13.0
>Reporter: Bill Burcham
>Assignee: Jinmei Liao
>Priority: Major
> Labels: GeodeOperationAPI
>
> As reported on the dev@geode mailing list
> https://markmail.org/message/qfnnj2s2x7dzbnzx and shown in the
> {{testRollingUpgradeOfGeodeWithGfs}} test in PR:
> https://github.com/apache/geode/pull/5224, if custom Jars are deployed, the
> locator doesn't start after a rolling upgrade and we see:
> {code}
> org.apache.geode.SerializationException: Could not
> create an instance of
> org.apache.geode.management.internal.configuration.domain.Configuration
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (GEODE-8245) CI failure: ClientServerMiscSelectorDUnitTest.testPingWrongServer fails with java.lang.AssertionError: expected:<1> but was:<0>
[
https://issues.apache.org/jira/browse/GEODE-8245?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138681#comment-17138681
]
John Hutchison commented on GEODE-8245:
---
note: error occured again in this ci build:
h6. DistributedTestOpenJDK11 #8376
> CI failure: ClientServerMiscSelectorDUnitTest.testPingWrongServer fails with
> java.lang.AssertionError: expected:<1> but was:<0>
> ---
>
> Key: GEODE-8245
> URL: https://issues.apache.org/jira/browse/GEODE-8245
> Project: Geode
> Issue Type: Bug
>Reporter: Kirk Lund
>Assignee: Alberto Bustamante Reyes
>Priority: Major
> Labels: flaky
>
> CI:
> http://files.apachegeode-ci.info/builds/apache-develop-main/1.14.0-build.0131/test-results/distributedTest/1591908487/
> {noformat}
> org.apache.geode.test.dunit.RMIException: While invoking
> org.apache.geode.internal.cache.tier.sockets.ClientServerMiscDUnitTestBase$$Lambda$431/0x000840c63c40.run
> in VM 3 running on Host e740fc204386 with 4 VMs
> at org.apache.geode.test.dunit.VM.executeMethodOnObject(VM.java:610)
> at org.apache.geode.test.dunit.VM.invoke(VM.java:437)
> at
> org.apache.geode.internal.cache.tier.sockets.ClientServerMiscDUnitTestBase.testPingWrongServer(ClientServerMiscDUnitTestBase.java:542)
> Caused by: java.lang.AssertionError: expected:<1> but was:<0>
> at org.junit.Assert.fail(Assert.java:88)
> at org.junit.Assert.failNotEquals(Assert.java:834)
> at org.junit.Assert.assertEquals(Assert.java:645)
> at org.junit.Assert.assertEquals(Assert.java:631)
> at
> org.apache.geode.internal.cache.tier.sockets.ClientServerMiscDUnitTestBase.lambda$testPingWrongServer$2200b406$2(ClientServerMiscDUnitTestBase.java:543)
> at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> at
> jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
> at
> jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> at java.lang.reflect.Method.invoke(Method.java:566)
> at
> org.apache.geode.test.dunit.internal.MethodInvoker.executeObject(MethodInvoker.java:123)
> at
> org.apache.geode.test.dunit.internal.RemoteDUnitVM.executeMethodOnObject(RemoteDUnitVM.java:78)
> {noformat}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (GEODE-8212) Change in PdxType class "<" operator impacts performance
[
https://issues.apache.org/jira/browse/GEODE-8212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138676#comment-17138676
]
ASF GitHub Bot commented on GEODE-8212:
---
alb3rtobr commented on a change in pull request #619:
URL: https://github.com/apache/geode-native/pull/619#discussion_r441715962
##
File path: cppcache/src/PdxType.cpp
##
@@ -557,11 +557,30 @@ bool PdxType::Equals(std::shared_ptr otherObj) {
}
bool PdxType::operator<(const PdxType& other) const {
- auto typeIdLessThan = this->m_geodeTypeId < other.m_geodeTypeId;
- auto typeIdsBothZero =
- (this->m_geodeTypeId == 0) && (other.m_geodeTypeId == 0);
- auto classnameLessThan = this->m_className < other.m_className;
- return (typeIdLessThan || (typeIdsBothZero && classnameLessThan));
+ if (m_geodeTypeId < other.m_geodeTypeId) {
+return true;
+ }
+
+ if ((m_geodeTypeId == 0) && (other.m_geodeTypeId == 0)) {
+return this->m_className < other.m_className;
+ }
+
+ return false;
+}
+
+int32_t PdxType::hashcode() const {
+ std::hash strHash;
+ auto result=strHash(this->m_className);
+
+ for (std::vector>::iterator it =
+ m_pdxFieldTypes->begin();
+ it != m_pdxFieldTypes->end(); ++it) {
+auto pdxPtr = *it;
+result = result ^ (strHash(pdxPtr->getClassName()) << 1 );
Review comment:
> Do we need to xor in the classname each time through the loop? It
seems like just using the field names in here would be more appropriate
I added it because I thought in the case of two different classes having the
same fields (names & types).
> One thing that is essential we get right here is that ordering of fields
should have no bearing on the generated hash. so { "foo": 7, "bar" 10 } is the
same type as { "bar": 12, "foo": 11 }, e.g. Otherwise we end up with a "type
explosion" of zillions of copies of the same thing each getting their own type
ID. If we're lucky, the server side will sort the field names when generating
the type ID on that side, and they'll all get the same ID, but then we've still
made a ton of unnecessary trips to the server.
In the tests I did, I saw that the order of the fields does not change the
result of the hash.
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:
us...@infra.apache.org
> Change in PdxType class "<" operator impacts performance
>
>
> Key: GEODE-8212
> URL: https://issues.apache.org/jira/browse/GEODE-8212
> Project: Geode
> Issue Type: Improvement
> Components: native client
>Reporter: Alberto Bustamante Reyes
>Assignee: Alberto Bustamante Reyes
>Priority: Major
> Attachments: performance_after_GEODE-7694.patch
>
>
> While investigating a performance problem of the C++ native client, it has
> been observed that change in the PdxType class "<" operator introduced by
> GEODE-7694 impacts performance of PdxHelper::serializePdx().
> Im attaching a patch with some code and a test case I have used to benchmark
> that method. With the current implementation in develop branch, creating just
> 10 instances is enough to see the difference. The calls to serializePdx take
> these times:
> {noformat}
> Elapsed Time (serializePdx): 32.1322 milliseconds
> Elapsed Time (serializePdx): 3.06044 milliseconds
> Elapsed Time (serializePdx): 1.58791 milliseconds
> Elapsed Time (serializePdx): 1.62466 milliseconds
> Elapsed Time (serializePdx): 1.53676 milliseconds
> Elapsed Time (serializePdx): 1.59278 milliseconds
> Elapsed Time (serializePdx): 1.82878 milliseconds
> Elapsed Time (serializePdx): 1.36811 milliseconds
> Elapsed Time (serializePdx): 1.6956 milliseconds
> Elapsed Time (serializePdx): 1.46873 milliseconds
> Elapsed Time (serializePdx): 1.53206 milliseconds
> {noformat}
> While these are the times using the old "<" operator:
> {noformat}
> Elapsed Time (serializePdx): 56.1524 milliseconds
> Elapsed Time (serializePdx): 0.012327 milliseconds
> Elapsed Time (serializePdx): 0.006325 milliseconds
> Elapsed Time (serializePdx): 0.005419 milliseconds
> Elapsed Time (serializePdx): 0.005266 milliseconds
> Elapsed Time (serializePdx): 0.005662 milliseconds
> Elapsed Time (serializePdx): 0.005407 milliseconds
> Elapsed Time (serializePdx): 0.005286 milliseconds
> Elapsed Time (serializePdx): 0.005467 milliseconds
> Elapsed Time (serializePdx): 0.005276 milliseconds
> Elapsed Time (serializePdx): 0.005298 milliseconds
> {noformat}
> And after creating 50.000 instances, these are the times for the last 10
> calls with the current "<" operator
[jira] [Updated] (GEODE-8200) Rebalance operations stuck in "IN_PROGRESS" state forever
[
https://issues.apache.org/jira/browse/GEODE-8200?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Aaron Lindsey updated GEODE-8200:
-
Attachment: GEODE-8200-exportedLogs.zip
> Rebalance operations stuck in "IN_PROGRESS" state forever
> -
>
> Key: GEODE-8200
> URL: https://issues.apache.org/jira/browse/GEODE-8200
> Project: Geode
> Issue Type: Bug
> Components: management
>Reporter: Aaron Lindsey
>Assignee: Jianxia Chen
>Priority: Major
> Labels: GeodeOperationAPI
> Attachments: GEODE-8200-exportedLogs.zip
>
>
> We use the management REST API to call rebalance immediately before stopping
> a server to limit the possibility of data loss. In a cluster with 3 locators,
> 3 servers, and no regions, we noticed that sometimes the rebalance operation
> never ends if one of the locators is restarting concurrently with the
> rebalance operation.
> More specifically, the scenario where we see this issue crop up is during an
> automated "rolling restart" operation in a Kubernetes environment which
> proceeds as follows:
> * At most one locator and one server are restarting at any point in time
> * Each locator/server waits until the previous locator/server is fully online
> before restarting
> * Immediately before stopping a server, a rebalance operation is performed
> and the server is not stopped until the rebalance operation is completed
> The impact of this issue is that the "rolling restart" operation will never
> complete, because it cannot proceed with stopping a server until the
> rebalance operation is completed. A human is then required to intervene and
> manually trigger a rebalance and stop the server. This type of "rolling
> restart" operation is triggered fairly often in Kubernetes — any time part of
> the configuration of the locators or servers changes.
> The following JSON is a sample response from the management REST API that
> shows the rebalance operation stuck in "IN_PROGRESS".
> {code}
> {
> "statusCode": "IN_PROGRESS",
> "links": {
> "self":
> "http://geodecluster-sample-locator.default/management/v1/operations/rebalances/a47f23c8-02b3-443c-a367-636fd6921ea7;,
> "list":
> "http://geodecluster-sample-locator.default/management/v1/operations/rebalances;
> },
> "operationStart": "2020-05-27T22:38:30.619Z",
> "operationId": "a47f23c8-02b3-443c-a367-636fd6921ea7",
> "operation": {
> "simulate": false
> }
> }
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Reopened] (GEODE-8095) Create restore redundancy and status redundancy REST API commands
[ https://issues.apache.org/jira/browse/GEODE-8095?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Mark Hanson reopened GEODE-8095: > Create restore redundancy and status redundancy REST API commands > - > > Key: GEODE-8095 > URL: https://issues.apache.org/jira/browse/GEODE-8095 > Project: Geode > Issue Type: Bug > Components: rest (admin) >Reporter: Anilkumar Gingade >Assignee: Mark Hanson >Priority: Major > Labels: GeodeOperationAPI > Fix For: 1.14.0 > > > Add two REST API commands to allow redundancy to be restored and to check the > current redundancy status. > Similar to: https://issues.apache.org/jira/browse/GEODE-7954 > More on restore redundancy can be found at > https://cwiki.apache.org/confluence/display/GEODE/Redundancy+Gfsh+Commands -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Assigned] (GEODE-8272) Refactor GFSH Restore Redundancy Command code to allow for REST API
[ https://issues.apache.org/jira/browse/GEODE-8272?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Mark Hanson reassigned GEODE-8272: -- Assignee: Mark Hanson > Refactor GFSH Restore Redundancy Command code to allow for REST API > --- > > Key: GEODE-8272 > URL: https://issues.apache.org/jira/browse/GEODE-8272 > Project: Geode > Issue Type: Bug > Components: gfsh >Reporter: Mark Hanson >Assignee: Mark Hanson >Priority: Major > > Refactor GFSH Restore Redundancy Command code to allow for REST API > > The core code path is specific to GFSH right now and needs a few changes to > support the REST API command. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Assigned] (GEODE-8095) Create restore redundancy and status redundancy REST API commands
[ https://issues.apache.org/jira/browse/GEODE-8095?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Mark Hanson reassigned GEODE-8095: -- Assignee: Mark Hanson > Create restore redundancy and status redundancy REST API commands > - > > Key: GEODE-8095 > URL: https://issues.apache.org/jira/browse/GEODE-8095 > Project: Geode > Issue Type: Bug > Components: rest (admin) >Reporter: Anilkumar Gingade >Assignee: Mark Hanson >Priority: Major > Labels: GeodeOperationAPI > Fix For: 1.14.0 > > > Add two REST API commands to allow redundancy to be restored and to check the > current redundancy status. > Similar to: https://issues.apache.org/jira/browse/GEODE-7954 > More on restore redundancy can be found at > https://cwiki.apache.org/confluence/display/GEODE/Redundancy+Gfsh+Commands -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (GEODE-8272) Refactor GFSH Restore Redundancy Command code to allow for REST API
Mark Hanson created GEODE-8272: -- Summary: Refactor GFSH Restore Redundancy Command code to allow for REST API Key: GEODE-8272 URL: https://issues.apache.org/jira/browse/GEODE-8272 Project: Geode Issue Type: Bug Components: gfsh Reporter: Mark Hanson Refactor GFSH Restore Redundancy Command code to allow for REST API The core code path is specific to GFSH right now and needs a few changes to support the REST API command. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-8259) DSFIDSerializerImpl should handle RTE the same as Exception
[
https://issues.apache.org/jira/browse/GEODE-8259?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138632#comment-17138632
]
ASF GitHub Bot commented on GEODE-8259:
---
bschuchardt commented on a change in pull request #5253:
URL: https://github.com/apache/geode/pull/5253#discussion_r441676962
##
File path:
geode-core/src/main/java/org/apache/geode/cache/client/internal/OpExecutorImpl.java
##
@@ -556,8 +556,8 @@ protected void handleException(Throwable e, Connection
conn, int retryCount, boo
title = null;
exToThrow = new ServerOperationException(e);
} else if (e instanceof SerializationException) {
- title = null; // no message
- exToThrow = new ServerOperationException(e);
+ title = "Unexpected SerializationException";
+ // exToThrow = new ServerOperationException(e);
Review comment:
you should remove this commented-out code
##
File path:
geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java
##
@@ -2506,153 +2506,156 @@ public static Object basicReadObject(final DataInput
in)
throw new IOException("Unknown header byte: " + header);
}
-switch (headerDSCode) {
- case DS_FIXED_ID_BYTE:
-return dsfidFactory.create(in.readByte(), in);
- case DS_FIXED_ID_SHORT:
-return dsfidFactory.create(in.readShort(), in);
- case DS_FIXED_ID_INT:
-return dsfidFactory.create(in.readInt(), in);
- case DS_NO_FIXED_ID:
- case DATA_SERIALIZABLE:
-return readDataSerializable(in);
- case NULL:
- case NULL_STRING:
-return null;
- case STRING:
-return readStringUTFFromDataInput(in);
- case HUGE_STRING:
-return readHugeStringFromDataInput(in);
- case STRING_BYTES:
-return readStringBytesFromDataInput(in, in.readUnsignedShort());
- case HUGE_STRING_BYTES:
-return readStringBytesFromDataInput(in, in.readInt());
- case CLASS:
-return readClass(in);
- case DATE:
-return readDate(in);
- case FILE:
-return readFile(in);
- case INET_ADDRESS:
-return readInetAddress(in);
- case BOOLEAN:
-return readBoolean(in);
- case CHARACTER:
-return readCharacter(in);
- case BYTE:
-return readByte(in);
- case SHORT:
-return readShort(in);
- case INTEGER:
-return readInteger(in);
- case LONG:
-return readLong(in);
- case FLOAT:
-return readFloat(in);
- case DOUBLE:
-return readDouble(in);
- case BYTE_ARRAY:
-return readByteArray(in);
- case ARRAY_OF_BYTE_ARRAYS:
-return readArrayOfByteArrays(in);
- case SHORT_ARRAY:
-return readShortArray(in);
- case STRING_ARRAY:
-return readStringArray(in);
- case INT_ARRAY:
-return readIntArray(in);
- case LONG_ARRAY:
-return readLongArray(in);
- case FLOAT_ARRAY:
-return readFloatArray(in);
- case DOUBLE_ARRAY:
-return readDoubleArray(in);
- case BOOLEAN_ARRAY:
-return readBooleanArray(in);
- case CHAR_ARRAY:
-return readCharArray(in);
- case OBJECT_ARRAY:
-return readObjectArray(in);
- case ARRAY_LIST:
-return readArrayList(in);
- case LINKED_LIST:
-return readLinkedList(in);
- case HASH_SET:
-return readHashSet(in);
- case LINKED_HASH_SET:
-return readLinkedHashSet(in);
- case HASH_MAP:
-return readHashMap(in);
- case IDENTITY_HASH_MAP:
-return readIdentityHashMap(in);
- case HASH_TABLE:
-return readHashtable(in);
- case CONCURRENT_HASH_MAP:
-return readConcurrentHashMap(in);
- case PROPERTIES:
-return readProperties(in);
- case TIME_UNIT:
-return readTimeUnit(in);
- case USER_CLASS:
-return readUserObject(in, in.readByte());
- case USER_CLASS_2:
-return readUserObject(in, in.readShort());
- case USER_CLASS_4:
-return readUserObject(in, in.readInt());
- case VECTOR:
-return readVector(in);
- case STACK:
-return readStack(in);
- case TREE_MAP:
-return readTreeMap(in);
- case TREE_SET:
-return readTreeSet(in);
- case BOOLEAN_TYPE:
-return Boolean.TYPE;
- case CHARACTER_TYPE:
-return Character.TYPE;
- case BYTE_TYPE:
-return Byte.TYPE;
- case SHORT_TYPE:
-return Short.TYPE;
- case INTEGER_TYPE:
-return Integer.TYPE;
- case LONG_TYPE:
-return Long.TYPE;
- case FLOAT_TYPE:
-return Float.TYPE;
- case DOUBLE_TYPE:
-return Double.TYPE;
- case VOID_TYPE:
-return Void.TYPE;
- case USER_DATA_SERIALIZABLE:
-return readUserDataSerializable(in, in.readByte());
-
[jira] [Commented] (GEODE-6903) CI Failure: GemFireTransactionDataSourceIntegrationTest.testExceptionHandlingGetConnection failed with Assertion
[
https://issues.apache.org/jira/browse/GEODE-6903?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138629#comment-17138629
]
Eric Shu commented on GEODE-6903:
-
Failed again in
https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/WindowsCoreIntegrationTestOpenJDK8/builds/266#A
> CI Failure:
> GemFireTransactionDataSourceIntegrationTest.testExceptionHandlingGetConnection
> failed with Assertion
>
>
> Key: GEODE-6903
> URL: https://issues.apache.org/jira/browse/GEODE-6903
> Project: Geode
> Issue Type: Bug
> Components: transactions
>Reporter: Eric Shu
>Assignee: Kirk Lund
>Priority: Major
> Labels: flaky
>
> {noformat}
> org.apache.geode.internal.datasource.GemFireTransactionDataSourceIntegrationTest
> > testExceptionHandlingGetConnection FAILED
> org.junit.ComparisonFailure: expected:<[0]> but was:<[2]>
> at sun.reflect.GeneratedConstructorAccessor26.newInstance(Unknown
> Source)
> at
> sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
> at
> org.apache.geode.internal.datasource.GemFireTransactionDataSourceIntegrationTest.testExceptionHandlingGetConnection(GemFireTransactionDataSourceIntegrationTest.java:141)
> {noformat}
> =-=-=-=-=-=-=-=-=-=-=-=-=-=-= Test Results URI
> =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
> http://files.apachegeode-ci.info/builds/apache-develop-main/1.10.0-SNAPSHOT.0399/test-results/integrationTest/1561170841/
> =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
> Test report artifacts from this job are available at:
> http://files.apachegeode-ci.info/builds/apache-develop-main/1.10.0-SNAPSHOT.0399/test-artifacts/1561170841/integrationtestfiles-OpenJDK8-1.10.0-SNAPSHOT.0399.tgz
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Assigned] (GEODE-8270) Reorganize Redis API for Geode test packages to match main package structure
[ https://issues.apache.org/jira/browse/GEODE-8270?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Darrel Schneider reassigned GEODE-8270: --- Assignee: (was: Darrel Schneider) > Reorganize Redis API for Geode test packages to match main package structure > > > Key: GEODE-8270 > URL: https://issues.apache.org/jira/browse/GEODE-8270 > Project: Geode > Issue Type: Improvement > Components: redis >Reporter: Sarah Abbey >Priority: Major > > Reorganize test packages to match main package structure so it's easier to > find relevant tests -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Assigned] (GEODE-8270) Reorganize Redis API for Geode test packages to match main package structure
[ https://issues.apache.org/jira/browse/GEODE-8270?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Darrel Schneider reassigned GEODE-8270: --- Assignee: Darrel Schneider > Reorganize Redis API for Geode test packages to match main package structure > > > Key: GEODE-8270 > URL: https://issues.apache.org/jira/browse/GEODE-8270 > Project: Geode > Issue Type: Improvement > Components: redis >Reporter: Sarah Abbey >Assignee: Darrel Schneider >Priority: Major > > Reorganize test packages to match main package structure so it's easier to > find relevant tests -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-8271) Update Spring dependencies to newer patch versions
[ https://issues.apache.org/jira/browse/GEODE-8271?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138610#comment-17138610 ] ASF GitHub Bot commented on GEODE-8271: --- yozaner1324 opened a new pull request #5264: URL: https://github.com/apache/geode/pull/5264 Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, check Concourse for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to d...@geode.apache.org. 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: us...@infra.apache.org > Update Spring dependencies to newer patch versions > -- > > Key: GEODE-8271 > URL: https://issues.apache.org/jira/browse/GEODE-8271 > Project: Geode > Issue Type: Improvement >Reporter: Patrick Johnsn >Assignee: Patrick Johnsn >Priority: Major > > Update all Spring artifacts to the latest patch versions. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (GEODE-8271) Update Spring dependencies to newer patch versions
Patrick Johnsn created GEODE-8271: - Summary: Update Spring dependencies to newer patch versions Key: GEODE-8271 URL: https://issues.apache.org/jira/browse/GEODE-8271 Project: Geode Issue Type: Improvement Reporter: Patrick Johnsn Update all Spring artifacts to the latest patch versions. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Assigned] (GEODE-8271) Update Spring dependencies to newer patch versions
[ https://issues.apache.org/jira/browse/GEODE-8271?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Patrick Johnsn reassigned GEODE-8271: - Assignee: Patrick Johnsn > Update Spring dependencies to newer patch versions > -- > > Key: GEODE-8271 > URL: https://issues.apache.org/jira/browse/GEODE-8271 > Project: Geode > Issue Type: Improvement >Reporter: Patrick Johnsn >Assignee: Patrick Johnsn >Priority: Major > > Update all Spring artifacts to the latest patch versions. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (GEODE-8270) Reorganize Redis API for Geode test packages to match main package structure
[ https://issues.apache.org/jira/browse/GEODE-8270?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Sarah Abbey updated GEODE-8270: --- Summary: Reorganize Redis API for Geode test packages to match main package structure (was: Reorganize test packages to match main package structure) > Reorganize Redis API for Geode test packages to match main package structure > > > Key: GEODE-8270 > URL: https://issues.apache.org/jira/browse/GEODE-8270 > Project: Geode > Issue Type: Improvement >Reporter: Sarah Abbey >Priority: Major > > Reorganize test packages to match main package structure so it's easier to > find relevant tests -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-8270) Reorganize Redis API for Geode test packages to match main package structure
[ https://issues.apache.org/jira/browse/GEODE-8270?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138609#comment-17138609 ] ASF GitHub Bot commented on GEODE-8270: --- sabbeyPivotal opened a new pull request #5263: URL: https://github.com/apache/geode/pull/5263 Reorganize test packages to match main package structure so it's easier to find relevant tests 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: us...@infra.apache.org > Reorganize Redis API for Geode test packages to match main package structure > > > Key: GEODE-8270 > URL: https://issues.apache.org/jira/browse/GEODE-8270 > Project: Geode > Issue Type: Improvement >Reporter: Sarah Abbey >Priority: Major > > Reorganize test packages to match main package structure so it's easier to > find relevant tests -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (GEODE-8270) Reorganize Redis API for Geode test packages to match main package structure
[ https://issues.apache.org/jira/browse/GEODE-8270?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Sarah Abbey updated GEODE-8270: --- Component/s: redis > Reorganize Redis API for Geode test packages to match main package structure > > > Key: GEODE-8270 > URL: https://issues.apache.org/jira/browse/GEODE-8270 > Project: Geode > Issue Type: Improvement > Components: redis >Reporter: Sarah Abbey >Priority: Major > > Reorganize test packages to match main package structure so it's easier to > find relevant tests -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-8251) exception creating domain.Configuration stops locator startup after rolling upgrade
[
https://issues.apache.org/jira/browse/GEODE-8251?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138585#comment-17138585
]
ASF GitHub Bot commented on GEODE-8251:
---
jinmeiliao commented on pull request #5257:
URL: https://github.com/apache/geode/pull/5257#issuecomment-645455002
> > > What's the point for these changes?
> > > I have ran the test case without them and it passes.
> > > Also, I have searched in the Geode code for the use of this file and
have only found it in `ObjectInputStreamFilterWrapper.java`. From what I see
there, only the first part of every line, the class name, is used.
> > > What am I missing?
> >
> >
> > the previous version of Deployment has variable name "jarFileName"
instead of "fileName", when we are deserializing a previous version bytes, if
the variable name doesn't match, it won't take the value. We should have a test
to verify that, but I saw the behavior in the debugger.
>
> I understand that. What I do not get are the changes in the
>
> > > What's the point for these changes?
> > > I have ran the test case without them and it passes.
> > > Also, I have searched in the Geode code for the use of this file and
have only found it in `ObjectInputStreamFilterWrapper.java`. From what I see
there, only the first part of every line, the class name, is used.
> > > What am I missing?
> >
> >
> > the previous version of Deployment has variable name "jarFileName"
instead of "fileName", when we are deserializing a previous version bytes, if
the variable name doesn't match, it won't take the value. We should have a test
to verify that, but I saw the behavior in the debugger.
>
> I understand the change in the members of the class. What I do not get is
why it needs to be reflected in sanctioned-geode-management-serializables.txt.
> Who is accessing that file?
the `AnalyzeManagementSerializablesJUnitTest` is making sure that we don't
make random changes in those serialized classes. Every change in the sanction
list poses a potential upgrade issue. So the fact that the test is broken will
make the developer think twice about the changes he/she is making to the class.
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:
us...@infra.apache.org
> exception creating domain.Configuration stops locator startup after rolling
> upgrade
> ---
>
> Key: GEODE-8251
> URL: https://issues.apache.org/jira/browse/GEODE-8251
> Project: Geode
> Issue Type: Bug
> Components: configuration
>Affects Versions: 1.13.0
>Reporter: Bill Burcham
>Assignee: Jinmei Liao
>Priority: Major
> Labels: GeodeOperationAPI
>
> As reported on the dev@geode mailing list
> https://markmail.org/message/qfnnj2s2x7dzbnzx and shown in the
> {{testRollingUpgradeOfGeodeWithGfs}} test in PR:
> https://github.com/apache/geode/pull/5224, if custom Jars are deployed, the
> locator doesn't start after a rolling upgrade and we see:
> {code}
> org.apache.geode.SerializationException: Could not
> create an instance of
> org.apache.geode.management.internal.configuration.domain.Configuration
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Created] (GEODE-8270) Reorganize test packages to match main package structure
Sarah Abbey created GEODE-8270: -- Summary: Reorganize test packages to match main package structure Key: GEODE-8270 URL: https://issues.apache.org/jira/browse/GEODE-8270 Project: Geode Issue Type: Improvement Reporter: Sarah Abbey Reorganize test packages to match main package structure so it's easier to find relevant tests -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-8269) Improve test coverage
[
https://issues.apache.org/jira/browse/GEODE-8269?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138537#comment-17138537
]
ASF GitHub Bot commented on GEODE-8269:
---
sabbeyPivotal commented on a change in pull request #5262:
URL: https://github.com/apache/geode/pull/5262#discussion_r441615674
##
File path:
geode-redis/src/test/java/org/apache/geode/redis/internal/executor/key/ExpireAtExecutorJUnitTest.java
##
@@ -34,30 +37,30 @@
@Test
public void calledWithTooFewCommandArguments_returnsError() {
-List commandsAsBytesWithTooFewArguments = new ArrayList<>();
-commandsAsBytesWithTooFewArguments.add("EXPIREAT".getBytes());
-commandsAsBytesWithTooFewArguments.add("key".getBytes());
-Command command = new Command(commandsAsBytesWithTooFewArguments);
+List commandAsBytes = new ArrayList<>();
+commandAsBytes.add("EXPIREAT".getBytes());
+commandAsBytes.add("key".getBytes());
+Command command = new Command(commandAsBytes);
-RedisResponse response =
-new ExpireAtExecutor().executeCommand(command, mockContext());
+Throwable thrown = catchThrowable(() -> command.execute(mockContext()));
Review comment:
yeah! good catch.
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:
us...@infra.apache.org
> Improve test coverage
> -
>
> Key: GEODE-8269
> URL: https://issues.apache.org/jira/browse/GEODE-8269
> Project: Geode
> Issue Type: Test
> Components: redis, tests
>Reporter: Sarah Abbey
>Priority: Major
>
> Analyzed test coverage via Intellij and by looking through all tests. There
> were some test cases missing. We either added them or created additional
> stories in Tracker.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (GEODE-8269) Improve test coverage
[
https://issues.apache.org/jira/browse/GEODE-8269?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138538#comment-17138538
]
ASF GitHub Bot commented on GEODE-8269:
---
sabbeyPivotal commented on a change in pull request #5262:
URL: https://github.com/apache/geode/pull/5262#discussion_r441615964
##
File path:
geode-redis/src/test/java/org/apache/geode/redis/internal/executor/key/ExpireAtExecutorJUnitTest.java
##
@@ -34,30 +37,30 @@
@Test
public void calledWithTooFewCommandArguments_returnsError() {
-List commandsAsBytesWithTooFewArguments = new ArrayList<>();
-commandsAsBytesWithTooFewArguments.add("EXPIREAT".getBytes());
-commandsAsBytesWithTooFewArguments.add("key".getBytes());
-Command command = new Command(commandsAsBytesWithTooFewArguments);
+List commandAsBytes = new ArrayList<>();
+commandAsBytes.add("EXPIREAT".getBytes());
+commandAsBytes.add("key".getBytes());
+Command command = new Command(commandAsBytes);
-RedisResponse response =
-new ExpireAtExecutor().executeCommand(command, mockContext());
+Throwable thrown = catchThrowable(() -> command.execute(mockContext()));
-assertThat(response.toString()).startsWith("-ERR The wrong number of
arguments");
+AssertionsForClassTypes.assertThat(thrown).hasMessageContaining("wrong
number of arguments");
+
AssertionsForClassTypes.assertThat(thrown).isInstanceOf(RedisParametersMismatchException.class);
}
@Test
public void calledWithTooManyCommandArguments_returnsError() {
-List commandsAsBytesWithTooFewArguments = new ArrayList<>();
-commandsAsBytesWithTooFewArguments.add("EXPIREAT".getBytes());
-commandsAsBytesWithTooFewArguments.add("key".getBytes());
-commandsAsBytesWithTooFewArguments.add("1".getBytes());
-commandsAsBytesWithTooFewArguments.add("extra-argument".getBytes());
-Command command = new Command(commandsAsBytesWithTooFewArguments);
+List commandsAsBytes = new ArrayList<>();
+commandsAsBytes.add("EXPIREAT".getBytes());
+commandsAsBytes.add("key".getBytes());
+commandsAsBytes.add("1".getBytes());
+commandsAsBytes.add("extra-argument".getBytes());
+Command command = new Command(commandsAsBytes);
-RedisResponse response =
-new ExpireAtExecutor().executeCommand(command, mockContext());
+Throwable thrown = catchThrowable(() -> command.execute(mockContext()));
-assertThat(response.toString()).startsWith("-ERR The wrong number of
arguments");
+AssertionsForClassTypes.assertThat(thrown).hasMessageContaining("wrong
number of arguments");
+
AssertionsForClassTypes.assertThat(thrown).isInstanceOf(RedisParametersMismatchException.class);
Review comment:
Definitely. Editing those now.
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:
us...@infra.apache.org
> Improve test coverage
> -
>
> Key: GEODE-8269
> URL: https://issues.apache.org/jira/browse/GEODE-8269
> Project: Geode
> Issue Type: Test
> Components: redis, tests
>Reporter: Sarah Abbey
>Priority: Major
>
> Analyzed test coverage via Intellij and by looking through all tests. There
> were some test cases missing. We either added them or created additional
> stories in Tracker.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (GEODE-8251) exception creating domain.Configuration stops locator startup after rolling upgrade
[
https://issues.apache.org/jira/browse/GEODE-8251?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138522#comment-17138522
]
ASF GitHub Bot commented on GEODE-8251:
---
albertogpz commented on pull request #5257:
URL: https://github.com/apache/geode/pull/5257#issuecomment-645426599
> > What's the point for these changes?
> > I have ran the test case without them and it passes.
> > Also, I have searched in the Geode code for the use of this file and
have only found it in `ObjectInputStreamFilterWrapper.java`. From what I see
there, only the first part of every line, the class name, is used.
> > What am I missing?
>
> the previous version of Deployment has variable name "jarFileName" instead
of "fileName", when we are deserializing a previous version bytes, if the
variable name doesn't match, it won't take the value. We should have a test to
verify that, but I saw the behavior in the debugger.
I understand that. What I do not get are the changes in the
> > What's the point for these changes?
> > I have ran the test case without them and it passes.
> > Also, I have searched in the Geode code for the use of this file and
have only found it in `ObjectInputStreamFilterWrapper.java`. From what I see
there, only the first part of every line, the class name, is used.
> > What am I missing?
>
> the previous version of Deployment has variable name "jarFileName" instead
of "fileName", when we are deserializing a previous version bytes, if the
variable name doesn't match, it won't take the value. We should have a test to
verify that, but I saw the behavior in the debugger.
I understand the change in the members of the class. What I do not get is
why it needs to be reflected in sanctioned-geode-management-serializables.txt.
Who is accessing that file?
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:
us...@infra.apache.org
> exception creating domain.Configuration stops locator startup after rolling
> upgrade
> ---
>
> Key: GEODE-8251
> URL: https://issues.apache.org/jira/browse/GEODE-8251
> Project: Geode
> Issue Type: Bug
> Components: configuration
>Affects Versions: 1.13.0
>Reporter: Bill Burcham
>Assignee: Jinmei Liao
>Priority: Major
> Labels: GeodeOperationAPI
>
> As reported on the dev@geode mailing list
> https://markmail.org/message/qfnnj2s2x7dzbnzx and shown in the
> {{testRollingUpgradeOfGeodeWithGfs}} test in PR:
> https://github.com/apache/geode/pull/5224, if custom Jars are deployed, the
> locator doesn't start after a rolling upgrade and we see:
> {code}
> org.apache.geode.SerializationException: Could not
> create an instance of
> org.apache.geode.management.internal.configuration.domain.Configuration
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Closed] (GEODE-8262) Eliminate use of master/slave terminology in old test framework
[ https://issues.apache.org/jira/browse/GEODE-8262?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Blake Bender closed GEODE-8262. --- > Eliminate use of master/slave terminology in old test framework > --- > > Key: GEODE-8262 > URL: https://issues.apache.org/jira/browse/GEODE-8262 > Project: Geode > Issue Type: Improvement > Components: native client >Reporter: Blake Bender >Assignee: Blake Bender >Priority: Major > > I ran across this again while debugging, and each time I do I find it > jarring. Ultimately the plan is to remove the old framework altogether, but > I think if we're being honest we have to admit this really isn't going away > anytime soon. In the meantime, this is is clearly the right thing to do, and > is confined to a very few source files in the old C++ test framework, so > changing these terms costs us nearly nothing. Let's scrub it ASAP. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Resolved] (GEODE-8262) Eliminate use of master/slave terminology in old test framework
[ https://issues.apache.org/jira/browse/GEODE-8262?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Blake Bender resolved GEODE-8262. - Resolution: Fixed > Eliminate use of master/slave terminology in old test framework > --- > > Key: GEODE-8262 > URL: https://issues.apache.org/jira/browse/GEODE-8262 > Project: Geode > Issue Type: Improvement > Components: native client >Reporter: Blake Bender >Assignee: Blake Bender >Priority: Major > > I ran across this again while debugging, and each time I do I find it > jarring. Ultimately the plan is to remove the old framework altogether, but > I think if we're being honest we have to admit this really isn't going away > anytime soon. In the meantime, this is is clearly the right thing to do, and > is confined to a very few source files in the old C++ test framework, so > changing these terms costs us nearly nothing. Let's scrub it ASAP. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-8262) Eliminate use of master/slave terminology in old test framework
[ https://issues.apache.org/jira/browse/GEODE-8262?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138506#comment-17138506 ] ASF GitHub Bot commented on GEODE-8262: --- pdxcodemonkey merged pull request #620: URL: https://github.com/apache/geode-native/pull/620 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: us...@infra.apache.org > Eliminate use of master/slave terminology in old test framework > --- > > Key: GEODE-8262 > URL: https://issues.apache.org/jira/browse/GEODE-8262 > Project: Geode > Issue Type: Improvement > Components: native client >Reporter: Blake Bender >Assignee: Blake Bender >Priority: Major > > I ran across this again while debugging, and each time I do I find it > jarring. Ultimately the plan is to remove the old framework altogether, but > I think if we're being honest we have to admit this really isn't going away > anytime soon. In the meantime, this is is clearly the right thing to do, and > is confined to a very few source files in the old C++ test framework, so > changing these terms costs us nearly nothing. Let's scrub it ASAP. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-8262) Eliminate use of master/slave terminology in old test framework
[ https://issues.apache.org/jira/browse/GEODE-8262?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138504#comment-17138504 ] ASF subversion and git services commented on GEODE-8262: Commit 8aff9a0750624bca9744748e7be029f4a4a306f9 in geode-native's branch refs/heads/develop from Blake Bender [ https://gitbox.apache.org/repos/asf?p=geode-native.git;h=8aff9a0 ] GEODE-8262: Replace insensitive terms in dunit framework (#620) - master/slave --> coordinator/worker > Eliminate use of master/slave terminology in old test framework > --- > > Key: GEODE-8262 > URL: https://issues.apache.org/jira/browse/GEODE-8262 > Project: Geode > Issue Type: Improvement > Components: native client >Reporter: Blake Bender >Assignee: Blake Bender >Priority: Major > > I ran across this again while debugging, and each time I do I find it > jarring. Ultimately the plan is to remove the old framework altogether, but > I think if we're being honest we have to admit this really isn't going away > anytime soon. In the meantime, this is is clearly the right thing to do, and > is confined to a very few source files in the old C++ test framework, so > changing these terms costs us nearly nothing. Let's scrub it ASAP. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-8251) exception creating domain.Configuration stops locator startup after rolling upgrade
[
https://issues.apache.org/jira/browse/GEODE-8251?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138489#comment-17138489
]
ASF GitHub Bot commented on GEODE-8251:
---
jinmeiliao commented on pull request #5257:
URL: https://github.com/apache/geode/pull/5257#issuecomment-645409764
@albertogpz the pipeline tests failed because we added GEODE_HOME
environment variable in all tests environment, some tests are able to find the
war files thus starting the web services, thus getting the address bind
exception. Is there any way we can add that environment variable only to one
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> exception creating domain.Configuration stops locator startup after rolling
> upgrade
> ---
>
> Key: GEODE-8251
> URL: https://issues.apache.org/jira/browse/GEODE-8251
> Project: Geode
> Issue Type: Bug
> Components: configuration
>Affects Versions: 1.13.0
>Reporter: Bill Burcham
>Assignee: Jinmei Liao
>Priority: Major
> Labels: GeodeOperationAPI
>
> As reported on the dev@geode mailing list
> https://markmail.org/message/qfnnj2s2x7dzbnzx and shown in the
> {{testRollingUpgradeOfGeodeWithGfs}} test in PR:
> https://github.com/apache/geode/pull/5224, if custom Jars are deployed, the
> locator doesn't start after a rolling upgrade and we see:
> {code}
> org.apache.geode.SerializationException: Could not
> create an instance of
> org.apache.geode.management.internal.configuration.domain.Configuration
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (GEODE-8251) exception creating domain.Configuration stops locator startup after rolling upgrade
[
https://issues.apache.org/jira/browse/GEODE-8251?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138486#comment-17138486
]
ASF GitHub Bot commented on GEODE-8251:
---
jinmeiliao commented on pull request #5257:
URL: https://github.com/apache/geode/pull/5257#issuecomment-645408502
> What's the point for these changes?
> I have ran the test case without them and it passes.
> Also, I have searched in the Geode code for the use of this file and have
only found it in `ObjectInputStreamFilterWrapper.java`. From what I see there,
only the first part of every line, the class name, is used.
> What am I missing?
the previous version of Deployment has variable name "jarFileName" instead
of "fileName", when we are deserializing a previous version bytes, if the
variable name doesn't match, it won't take the value. We should have a test to
verify that, but I saw the behavior in the debugger.
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:
us...@infra.apache.org
> exception creating domain.Configuration stops locator startup after rolling
> upgrade
> ---
>
> Key: GEODE-8251
> URL: https://issues.apache.org/jira/browse/GEODE-8251
> Project: Geode
> Issue Type: Bug
> Components: configuration
>Affects Versions: 1.13.0
>Reporter: Bill Burcham
>Assignee: Jinmei Liao
>Priority: Major
> Labels: GeodeOperationAPI
>
> As reported on the dev@geode mailing list
> https://markmail.org/message/qfnnj2s2x7dzbnzx and shown in the
> {{testRollingUpgradeOfGeodeWithGfs}} test in PR:
> https://github.com/apache/geode/pull/5224, if custom Jars are deployed, the
> locator doesn't start after a rolling upgrade and we see:
> {code}
> org.apache.geode.SerializationException: Could not
> create an instance of
> org.apache.geode.management.internal.configuration.domain.Configuration
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (GEODE-8269) Improve test coverage
[
https://issues.apache.org/jira/browse/GEODE-8269?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138476#comment-17138476
]
ASF GitHub Bot commented on GEODE-8269:
---
jdeppe-pivotal commented on a change in pull request #5262:
URL: https://github.com/apache/geode/pull/5262#discussion_r441579410
##
File path:
geode-redis/src/test/java/org/apache/geode/redis/internal/executor/key/ExpireAtExecutorJUnitTest.java
##
@@ -34,30 +37,30 @@
@Test
public void calledWithTooFewCommandArguments_returnsError() {
-List commandsAsBytesWithTooFewArguments = new ArrayList<>();
-commandsAsBytesWithTooFewArguments.add("EXPIREAT".getBytes());
-commandsAsBytesWithTooFewArguments.add("key".getBytes());
-Command command = new Command(commandsAsBytesWithTooFewArguments);
+List commandAsBytes = new ArrayList<>();
+commandAsBytes.add("EXPIREAT".getBytes());
+commandAsBytes.add("key".getBytes());
+Command command = new Command(commandAsBytes);
-RedisResponse response =
-new ExpireAtExecutor().executeCommand(command, mockContext());
+Throwable thrown = catchThrowable(() -> command.execute(mockContext()));
-assertThat(response.toString()).startsWith("-ERR The wrong number of
arguments");
+AssertionsForClassTypes.assertThat(thrown).hasMessageContaining("wrong
number of arguments");
+
AssertionsForClassTypes.assertThat(thrown).isInstanceOf(RedisParametersMismatchException.class);
}
@Test
public void calledWithTooManyCommandArguments_returnsError() {
-List commandsAsBytesWithTooFewArguments = new ArrayList<>();
-commandsAsBytesWithTooFewArguments.add("EXPIREAT".getBytes());
-commandsAsBytesWithTooFewArguments.add("key".getBytes());
-commandsAsBytesWithTooFewArguments.add("1".getBytes());
-commandsAsBytesWithTooFewArguments.add("extra-argument".getBytes());
-Command command = new Command(commandsAsBytesWithTooFewArguments);
+List commandsAsBytes = new ArrayList<>();
+commandsAsBytes.add("EXPIREAT".getBytes());
+commandsAsBytes.add("key".getBytes());
+commandsAsBytes.add("1".getBytes());
+commandsAsBytes.add("extra-argument".getBytes());
+Command command = new Command(commandsAsBytes);
-RedisResponse response =
-new ExpireAtExecutor().executeCommand(command, mockContext());
+Throwable thrown = catchThrowable(() -> command.execute(mockContext()));
-assertThat(response.toString()).startsWith("-ERR The wrong number of
arguments");
+AssertionsForClassTypes.assertThat(thrown).hasMessageContaining("wrong
number of arguments");
+
AssertionsForClassTypes.assertThat(thrown).isInstanceOf(RedisParametersMismatchException.class);
Review comment:
Maybe the same thing, but at least for consistency, `assertThat` should
come from the `org.assertj.core.api.Assertions` package.
##
File path:
geode-redis/src/test/java/org/apache/geode/redis/internal/executor/key/KeysExecutorJUnitTest.java
##
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express
+ * or implied. See the License for the specific language governing permissions
and limitations under
+ * the License.
+ *
+ */
+
+package org.apache.geode.redis.internal.executor.key;
+
+import static org.assertj.core.api.AssertionsForClassTypes.catchThrowable;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import io.netty.buffer.UnpooledByteBufAllocator;
+import org.assertj.core.api.AssertionsForClassTypes;
+import org.junit.Test;
+
+import
org.apache.geode.redis.internal.ParameterRequirements.RedisParametersMismatchException;
+import org.apache.geode.redis.internal.netty.Command;
+import org.apache.geode.redis.internal.netty.ExecutionHandlerContext;
+
+
+public class KeysExecutorJUnitTest {
+
+ @Test
+ public void calledWithTooFewOptions_returnsError() {
+List commandsAsBytes = new ArrayList<>();
+commandsAsBytes.add("KEYS".getBytes());
+Command command = new Command(commandsAsBytes);
+
+Throwable thrown = catchThrowable(() -> command.execute(mockContext()));
+
+AssertionsForClassTypes.assertThat(thrown).hasMessageContaining("wrong
number of arguments");
+
[jira] [Commented] (GEODE-8269) Improve test coverage
[ https://issues.apache.org/jira/browse/GEODE-8269?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138440#comment-17138440 ] ASF GitHub Bot commented on GEODE-8269: --- sabbeyPivotal opened a new pull request #5262: URL: https://github.com/apache/geode/pull/5262 Analyzed test coverage via Intellij and by looking through all tests. There were some test cases missing. We either added them or created additional stories in Tracker. 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: us...@infra.apache.org > Improve test coverage > - > > Key: GEODE-8269 > URL: https://issues.apache.org/jira/browse/GEODE-8269 > Project: Geode > Issue Type: Test > Components: redis, tests >Reporter: Sarah Abbey >Priority: Major > > Analyzed test coverage via Intellij and by looking through all tests. There > were some test cases missing. We either added them or created additional > stories in Tracker. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (GEODE-8269) Improve test coverage
Sarah Abbey created GEODE-8269: -- Summary: Improve test coverage Key: GEODE-8269 URL: https://issues.apache.org/jira/browse/GEODE-8269 Project: Geode Issue Type: Test Components: redis, tests Reporter: Sarah Abbey Analyzed test coverage via Intellij and by looking through all tests. There were some test cases missing. We either added them or created additional stories in Tracker. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-8251) exception creating domain.Configuration stops locator startup after rolling upgrade
[
https://issues.apache.org/jira/browse/GEODE-8251?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138253#comment-17138253
]
ASF GitHub Bot commented on GEODE-8251:
---
albertogpz commented on a change in pull request #5257:
URL: https://github.com/apache/geode/pull/5257#discussion_r441382563
##
File path:
geode-core/src/upgradeTest/java/org/apache/geode/internal/cache/rollingupgrade/RollingUpgradeWithGfshDUnitTest.java
##
@@ -0,0 +1,160 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express
+ * or implied. See the License for the specific language governing permissions
and limitations under
+ * the License.
+ */
+package org.apache.geode.internal.cache.rollingupgrade;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express
+ * or implied. See the License for the specific language governing permissions
and limitations under
+ * the License.
+ */
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Collection;
+import java.util.List;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import org.apache.geode.internal.UniquePortSupplier;
+import org.apache.geode.test.compiler.ClassBuilder;
+import org.apache.geode.test.junit.categories.BackwardCompatibilityTest;
+import org.apache.geode.test.junit.rules.gfsh.GfshExecution;
+import org.apache.geode.test.junit.rules.gfsh.GfshRule;
+import org.apache.geode.test.junit.rules.gfsh.GfshScript;
+import
org.apache.geode.test.junit.runners.CategoryWithParameterizedRunnerFactory;
+import org.apache.geode.test.version.TestVersion;
+import org.apache.geode.test.version.VersionManager;
+
+/**
+ * This test iterates through the versions of Geode and executes client
compatibility with
+ * the current version of Geode.
+ */
+@Category({BackwardCompatibilityTest.class})
+@RunWith(Parameterized.class)
+@Parameterized.UseParametersRunnerFactory(CategoryWithParameterizedRunnerFactory.class)
+public class RollingUpgradeWithGfshDUnitTest {
+ private final UniquePortSupplier portSupplier = new UniquePortSupplier();
+ private final String oldVersion;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static Collection data() {
+List result =
VersionManager.getInstance().getVersionsWithoutCurrent();
+result.removeIf(s -> TestVersion.compare(s, "1.10.0") < 0);
Review comment:
Why start from this version? Is the upgrade from previous versions not
supported?
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:
us...@infra.apache.org
> exception creating domain.Configuration stops locator startup after rolling
> upgrade
> ---
>
> Key: GEODE-8251
> URL: https://issues.apache.org/jira/browse/GEODE-8251
> Project: Geode
> Issue Type: Bug
> Components: configuration
>Affects Versions: 1.13.0
>Reporter: Bill Burcham
>Assignee: Jinmei Liao
>Priority: Major
> Labels: GeodeOperationAPI
>
> As reported on the dev@geode mailing list
> https://markmail.org/message/qfnnj2s2x7dzbnzx and shown in the
> {{testRollingUpgradeOfGeodeWithGfs}} test in PR:
>
[jira] [Resolved] (GEODE-7591) potential hang
[
https://issues.apache.org/jira/browse/GEODE-7591?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jakov Varenina resolved GEODE-7591.
---
Fix Version/s: 1.14.0
Resolution: Fixed
> potential hang
> --
>
> Key: GEODE-7591
> URL: https://issues.apache.org/jira/browse/GEODE-7591
> Project: Geode
> Issue Type: Improvement
> Components: membership
>Reporter: Bruce J Schuchardt
>Assignee: Jakov Varenina
>Priority: Major
> Fix For: 1.14.0
>
>
> This method in ClusterDistributionManager waits for a new membership view to
> be installed, but if the cache is being closed while waiting the method could
> hang because it only checks for cache closure if the object it's waiting on
> is notified. We should change the wait() to have a timeout so that the
> `stopper` is polled periodically
> {code:java}
> void waitForViewInstallation(long id) throws InterruptedException {
> if (id <= membershipViewIdAcknowledged) {
> return;
> }
> synchronized (membershipViewIdGuard) {
> while (membershipViewIdAcknowledged < id &&
> !stopper.isCancelInProgress()) {
> if (logger.isDebugEnabled()) {
> logger.debug("waiting for view {}. Current DM view processed by all
> listeners is {}", id,
> membershipViewIdAcknowledged);
> }
> membershipViewIdGuard.wait();
> }
> }
> }
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (GEODE-7591) potential hang
[
https://issues.apache.org/jira/browse/GEODE-7591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138146#comment-17138146
]
ASF subversion and git services commented on GEODE-7591:
Commit 86778ecc5507aaeb28f05b069a80f766f705dbcd in geode's branch
refs/heads/develop from Jakov Varenina
[ https://gitbox.apache.org/repos/asf?p=geode.git;h=86778ec ]
GEODE-7591: Fix for hang in ClusterDistributionManager (#5182)
* GEODE-7591: Fix for hang in ClusterDistributionManager
Added timeout after which it will be periodically checked whether
distribution manager is disconnecting while waiting on the membership
view installation to avoid hanging
* DUnit test updated
* CyclicBarrier thread synchronization used instad await()
* Future used to wait for completion of test
* DUnit test updated
Exceptions added to test method signature
> potential hang
> --
>
> Key: GEODE-7591
> URL: https://issues.apache.org/jira/browse/GEODE-7591
> Project: Geode
> Issue Type: Improvement
> Components: membership
>Reporter: Bruce J Schuchardt
>Assignee: Jakov Varenina
>Priority: Major
>
> This method in ClusterDistributionManager waits for a new membership view to
> be installed, but if the cache is being closed while waiting the method could
> hang because it only checks for cache closure if the object it's waiting on
> is notified. We should change the wait() to have a timeout so that the
> `stopper` is polled periodically
> {code:java}
> void waitForViewInstallation(long id) throws InterruptedException {
> if (id <= membershipViewIdAcknowledged) {
> return;
> }
> synchronized (membershipViewIdGuard) {
> while (membershipViewIdAcknowledged < id &&
> !stopper.isCancelInProgress()) {
> if (logger.isDebugEnabled()) {
> logger.debug("waiting for view {}. Current DM view processed by all
> listeners is {}", id,
> membershipViewIdAcknowledged);
> }
> membershipViewIdGuard.wait();
> }
> }
> }
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (GEODE-7591) potential hang
[
https://issues.apache.org/jira/browse/GEODE-7591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138144#comment-17138144
]
ASF GitHub Bot commented on GEODE-7591:
---
mivanac merged pull request #5182:
URL: https://github.com/apache/geode/pull/5182
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:
us...@infra.apache.org
> potential hang
> --
>
> Key: GEODE-7591
> URL: https://issues.apache.org/jira/browse/GEODE-7591
> Project: Geode
> Issue Type: Improvement
> Components: membership
>Reporter: Bruce J Schuchardt
>Assignee: Jakov Varenina
>Priority: Major
>
> This method in ClusterDistributionManager waits for a new membership view to
> be installed, but if the cache is being closed while waiting the method could
> hang because it only checks for cache closure if the object it's waiting on
> is notified. We should change the wait() to have a timeout so that the
> `stopper` is polled periodically
> {code:java}
> void waitForViewInstallation(long id) throws InterruptedException {
> if (id <= membershipViewIdAcknowledged) {
> return;
> }
> synchronized (membershipViewIdGuard) {
> while (membershipViewIdAcknowledged < id &&
> !stopper.isCancelInProgress()) {
> if (logger.isDebugEnabled()) {
> logger.debug("waiting for view {}. Current DM view processed by all
> listeners is {}", id,
> membershipViewIdAcknowledged);
> }
> membershipViewIdGuard.wait();
> }
> }
> }
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (GEODE-7591) potential hang
[
https://issues.apache.org/jira/browse/GEODE-7591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138147#comment-17138147
]
ASF subversion and git services commented on GEODE-7591:
Commit 86778ecc5507aaeb28f05b069a80f766f705dbcd in geode's branch
refs/heads/develop from Jakov Varenina
[ https://gitbox.apache.org/repos/asf?p=geode.git;h=86778ec ]
GEODE-7591: Fix for hang in ClusterDistributionManager (#5182)
* GEODE-7591: Fix for hang in ClusterDistributionManager
Added timeout after which it will be periodically checked whether
distribution manager is disconnecting while waiting on the membership
view installation to avoid hanging
* DUnit test updated
* CyclicBarrier thread synchronization used instad await()
* Future used to wait for completion of test
* DUnit test updated
Exceptions added to test method signature
> potential hang
> --
>
> Key: GEODE-7591
> URL: https://issues.apache.org/jira/browse/GEODE-7591
> Project: Geode
> Issue Type: Improvement
> Components: membership
>Reporter: Bruce J Schuchardt
>Assignee: Jakov Varenina
>Priority: Major
>
> This method in ClusterDistributionManager waits for a new membership view to
> be installed, but if the cache is being closed while waiting the method could
> hang because it only checks for cache closure if the object it's waiting on
> is notified. We should change the wait() to have a timeout so that the
> `stopper` is polled periodically
> {code:java}
> void waitForViewInstallation(long id) throws InterruptedException {
> if (id <= membershipViewIdAcknowledged) {
> return;
> }
> synchronized (membershipViewIdGuard) {
> while (membershipViewIdAcknowledged < id &&
> !stopper.isCancelInProgress()) {
> if (logger.isDebugEnabled()) {
> logger.debug("waiting for view {}. Current DM view processed by all
> listeners is {}", id,
> membershipViewIdAcknowledged);
> }
> membershipViewIdGuard.wait();
> }
> }
> }
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Resolved] (GEODE-8268) redis ExecutionHandlerContext is doing more than it should
[ https://issues.apache.org/jira/browse/GEODE-8268?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Darrel Schneider resolved GEODE-8268. - Fix Version/s: 1.14.0 Resolution: Fixed > redis ExecutionHandlerContext is doing more than it should > -- > > Key: GEODE-8268 > URL: https://issues.apache.org/jira/browse/GEODE-8268 > Project: Geode > Issue Type: Improvement > Components: redis >Reporter: Darrel Schneider >Assignee: Darrel Schneider >Priority: Major > Fix For: 1.14.0 > > > ExecutionHandlerContext is supposed to provide context to redis command > executors so that they can execute their specific command. But in a couple of > cases (SHUTDOWN and SUBSCRIBE) ExecutionHandlerContext has code that is part > of the execution of a specific command. That code should be refactored so > that the specific command drives the execution of itself. > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-8268) redis ExecutionHandlerContext is doing more than it should
[ https://issues.apache.org/jira/browse/GEODE-8268?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138139#comment-17138139 ] ASF subversion and git services commented on GEODE-8268: Commit e159238175766b46cbb6fe1e3459aa2da68db756 in geode's branch refs/heads/develop from Darrel Schneider [ https://gitbox.apache.org/repos/asf?p=geode.git;h=e159238 ] GEODE-8268: clean up ExecutionHandlerContext (#5237) Cleaned up ExecutionHandlerContext dependencies. The ShutdownExecutor now implements the shutdown command instead of it being embedded in the ExecutionHandlerContext. The SubscribeExecutor now takes care of changing the EventGroup of its channel. > redis ExecutionHandlerContext is doing more than it should > -- > > Key: GEODE-8268 > URL: https://issues.apache.org/jira/browse/GEODE-8268 > Project: Geode > Issue Type: Improvement > Components: redis >Reporter: Darrel Schneider >Assignee: Darrel Schneider >Priority: Major > Fix For: 1.14.0 > > > ExecutionHandlerContext is supposed to provide context to redis command > executors so that they can execute their specific command. But in a couple of > cases (SHUTDOWN and SUBSCRIBE) ExecutionHandlerContext has code that is part > of the execution of a specific command. That code should be refactored so > that the specific command drives the execution of itself. > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-8268) redis ExecutionHandlerContext is doing more than it should
[ https://issues.apache.org/jira/browse/GEODE-8268?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17138136#comment-17138136 ] ASF GitHub Bot commented on GEODE-8268: --- dschneider-pivotal merged pull request #5237: URL: https://github.com/apache/geode/pull/5237 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: us...@infra.apache.org > redis ExecutionHandlerContext is doing more than it should > -- > > Key: GEODE-8268 > URL: https://issues.apache.org/jira/browse/GEODE-8268 > Project: Geode > Issue Type: Improvement > Components: redis >Reporter: Darrel Schneider >Assignee: Darrel Schneider >Priority: Major > > ExecutionHandlerContext is supposed to provide context to redis command > executors so that they can execute their specific command. But in a couple of > cases (SHUTDOWN and SUBSCRIBE) ExecutionHandlerContext has code that is part > of the execution of a specific command. That code should be refactored so > that the specific command drives the execution of itself. > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Assigned] (GEODE-8268) redis ExecutionHandlerContext is doing more than it should
[ https://issues.apache.org/jira/browse/GEODE-8268?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Darrel Schneider reassigned GEODE-8268: --- Assignee: Darrel Schneider > redis ExecutionHandlerContext is doing more than it should > -- > > Key: GEODE-8268 > URL: https://issues.apache.org/jira/browse/GEODE-8268 > Project: Geode > Issue Type: Improvement > Components: redis >Reporter: Darrel Schneider >Assignee: Darrel Schneider >Priority: Major > > ExecutionHandlerContext is supposed to provide context to redis command > executors so that they can execute their specific command. But in a couple of > cases (SHUTDOWN and SUBSCRIBE) ExecutionHandlerContext has code that is part > of the execution of a specific command. That code should be refactored so > that the specific command drives the execution of itself. > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (GEODE-8268) redis ExecutionHandlerContext is doing more than it should
Darrel Schneider created GEODE-8268: --- Summary: redis ExecutionHandlerContext is doing more than it should Key: GEODE-8268 URL: https://issues.apache.org/jira/browse/GEODE-8268 Project: Geode Issue Type: Improvement Components: redis Reporter: Darrel Schneider ExecutionHandlerContext is supposed to provide context to redis command executors so that they can execute their specific command. But in a couple of cases (SHUTDOWN and SUBSCRIBE) ExecutionHandlerContext has code that is part of the execution of a specific command. That code should be refactored so that the specific command drives the execution of itself. -- This message was sent by Atlassian Jira (v8.3.4#803005)
