[GitHub] geode pull request #677: GEODE-3038: A server process shuts down quietly whe...
Github user dschneider-pivotal commented on a diff in the pull request:
https://github.com/apache/geode/pull/677#discussion_r131003885
--- Diff:
geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
---
@@ -1208,6 +1208,9 @@ private void initialize() {
this.system.getConfig());
initializeDeclarativeCache();
completedCacheXml = true;
+} catch (CacheXmlException e) {
--- End diff --
It looks like a number of exceptions can be thrown from
initializeDeclarativeCache.
Would it be better to catch, log, and rethrow RuntimeException instead of
CacheXmlException?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #627: GEODE-3194: cleanup disk store on failed initial recovery
Github user dschneider-pivotal commented on the issue: https://github.com/apache/geode/pull/627 This looks good. I will pull it in. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #610: GEODE-3146 Remove doc reference to GemFire 8.2
Github user dschneider-pivotal commented on a diff in the pull request: https://github.com/apache/geode/pull/610#discussion_r124667734 --- Diff: geode-docs/developing/partitioned_regions/set_join_redundancy_recovery.html.md.erb --- @@ -19,23 +19,38 @@ See the License for the specific language governing permissions and limitations under the License. --> -Configure whether and how redundancy is recovered in a partition region after a member joins. +This section covers configuring whether and how redundancy is +recovered in a partitioned region, after a member joins. Use the partition attribute `startup-recovery-delay` to specify member join redundancy recovery. -| startup-recovery-delay partition attribute | Effect following a member join | +| value of `startup-recovery-delay`| Effect following a member join | ||--| -| -1 | No automatic recovery of redundancy after a new member comes online. If you use this and the default `recovery-delay` setting, you can only recover redundancy by kicking off rebalancing through a cacheserver or API call. | -| long greater than or equal to **0**| Number of milliseconds to wait after a member joins before before recovering redundancy. The default is 0 (zero), which causes immediate redundancy recovery whenever a new partitioned region host joins. | - -Setting this to a value higher than the default of 0 allows multiple new members to join before redundancy recovery kicks in. With the multiple members present during recovery, the system will spread redundancy recovery among them. With no delay, if multiple members are started in close succession, the system may choose only the first member started for most or all of the redundancy recovery. - -**Note:** -Satisfying redundancy is not the same as adding capacity. If redundancy is satisfied, new members do not take buckets until you invoke a rebalance. +| -1 | No automatic recovery of redundancy after a new member comes online. With this value and the default `recovery-delay` setting, redundancy recovery is only achieved by a rebalance operation. | +| long integer greater than or equal to **0** | Number of milliseconds to wait after a member joins before recovering redundancy. The default is 0 (zero), which causes immediate redundancy recovery whenever a member that hosts the partitioned region joins. | --- End diff -- I find "long integer greater" more confusing than the old "long greater". "long" and "integer" can both be used to indicate a specific Java ordinal type so having both of them is confusing. In this context I think ">=0" would work. If you want to keep it verbose perhaps changing the old "long" to something more generic like "number" or "value" would be better. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #577: Feature/geode 3049
Github user dschneider-pivotal commented on a diff in the pull request:
https://github.com/apache/geode/pull/577#discussion_r124134874
--- Diff:
geode-core/src/main/java/org/apache/geode/internal/cache/BucketRedundancyTracker.java
---
@@ -0,0 +1,110 @@
+/*
+ * 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;
+
+/**
+ * Keeps track of redundancy status for a bucket in a PartitionedRegion
and update the region's
+ * {@link PartitionedRegionRedundancyTracker} of the bucket's status for
the region.
+ */
+class BucketRedundancyTracker {
+ private boolean redundancySatisfied = false;
+ private boolean hasAnyCopies = false;
+ private boolean redundancyEverSatisfied = false;
+ private boolean hasEverHadCopies = false;
+ private int currentRedundancy = -1;
+ private final int targetRedundancy;
+ private final PartitionedRegionRedundancyTracker regionRedundancyTracker;
+
+ /**
+ * Creates a new BucketRedundancyTracker
+ *
+ * @param redundantCopies the number of redundant copies specified for
the
+ *{@link PartitionedRegion} of this bucket
+ * @param regionRedundancyTracker the redundancy tracker for the {@link
PartitionedRegion} of this
+ *bucket
+ */
+ BucketRedundancyTracker(int redundantCopies,
+ PartitionedRegionRedundancyTracker regionRedundancyTracker) {
+this.targetRedundancy = redundantCopies;
+this.regionRedundancyTracker = regionRedundancyTracker;
+ }
+
+ /**
+ * Adjust statistics based on closing a bucket
+ */
+ synchronized void closeBucket() {
+if (!redundancySatisfied) {
--- End diff --
Since "redundancySatisified" starts out false what prevents closeBucket
from decrementing the stat when it has never incremented it? Same question for
"hasAnyCopies". I can't tell that we will always increment these stats before
closeBucket is called.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #577: Feature/geode 3049
Github user dschneider-pivotal commented on a diff in the pull request:
https://github.com/apache/geode/pull/577#discussion_r121776319
--- Diff:
geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionRedundancyTracker.java
---
@@ -0,0 +1,133 @@
+/*
+ * 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;
+
+import org.apache.geode.internal.i18n.LocalizedStrings;
+import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.internal.logging.log4j.LocalizedMessage;
+import org.apache.logging.log4j.Logger;
+
+/**
+ * Keeps track redundancy statistics across the buckets of a given {@link
PartitionedRegion}
+ */
+public class PartitionedRegionRedundancyTracker {
+ private static final Logger logger = LogService.getLogger();
+
+ private final PartitionedRegionStats stats;
+ private final String regionPath;
+ private final int totalBuckets;
+ private final int targetRedundancy;
+
+ private int lowRedundancyBuckets;
+ private int noCopiesBuckets;
+ private int lowestBucketCopies;
+
+ /**
+ * Creates a new PartitionedRegionRedundancyTracker
+ *
+ * @param totalBuckets number of buckets in the region to track
+ * @param redundantCopies number of redundant copies specified on the
region to track
+ * @param stats the statistics container for the region to track
+ * @param regionPath the full path of the region to track
+ */
+ PartitionedRegionRedundancyTracker(int totalBuckets, int redundantCopies,
+ PartitionedRegionStats stats, String regionPath) {
+this.stats = stats;
+this.regionPath = regionPath;
+this.totalBuckets = totalBuckets;
+this.targetRedundancy = redundantCopies;
+this.lowestBucketCopies = redundantCopies + 1;
+ }
+
+ /**
+ * Since consistency was last reached, provides the lowest number of
copies of a bucket that have
+ * been remaining across all the buckets in the region
+ *
+ * @return the number of copies of the bucket with the least copies
available
+ */
+ int getLowestBucketCopies() {
+return lowestBucketCopies;
+ }
+
+ /**
+ * Increments the count of buckets that do not meet redundancy
+ */
+ synchronized void incrementLowRedundancyBucketCount() {
+if (lowRedundancyBuckets == totalBuckets) {
+ return;
+}
+lowRedundancyBuckets++;
+stats.incLowRedundancyBucketCount(1);
+ }
+
+ /**
+ * Updates the count of copies for the bucket with the least copies if a
new low has been reached
+ *
+ * @param bucketCopies number of copies of a bucket remaining
+ */
+ synchronized void reportBucketCount(int bucketCopies) {
+if (bucketCopies < lowestBucketCopies) {
+ lowestBucketCopies = bucketCopies;
+ logger.warn(LocalizedMessage.create(
+
LocalizedStrings.BucketAdvisor_REDUNDANCY_HAS_DROPPED_BELOW_0_CONFIGURED_COPIES_TO_1_ACTUAL_COPIES_FOR_2,
+ new Object[] {targetRedundancy, bucketCopies + 1, regionPath}));
+}
+ }
+
+ /**
+ * Increments the count of buckets that no longer have any copies
remaining
+ */
+ synchronized void incrementNoCopiesBucketCount() {
+if (noCopiesBuckets == totalBuckets) {
+ return;
+}
+noCopiesBuckets++;
+stats.incNoCopiesBucketCount(1);
+if (noCopiesBuckets == 1) {
+ logger.warn("No copies remain for " + regionPath);
--- End diff --
I think this warning needs to say more. We should let them know that it may
be ok but that data may have been lost
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #577: Feature/geode 3049
Github user dschneider-pivotal commented on a diff in the pull request:
https://github.com/apache/geode/pull/577#discussion_r121751707
--- Diff:
geode-core/src/main/java/org/apache/geode/internal/cache/BucketRedundancyTracker.java
---
@@ -0,0 +1,108 @@
+/*
+ * 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;
+
+/**
+ * Keeps track of redundancy status for a PartitionedRegion and (if
enabled) updates the statistics
+ * for the region.
+ */
+class BucketRedundancyTracker {
+ private boolean redundancySatisfied = false;
--- End diff --
It is not clear, from this class, what makes access to these non final non
volatile instance variables safe.
Should you add comments on the methods that modify them that those methods
are not thread safe and the caller is responsible for making sure they are not
concurrently called by multiple threads?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #577: Feature/geode 3049
Github user dschneider-pivotal commented on a diff in the pull request: https://github.com/apache/geode/pull/577#discussion_r121742105 --- Diff: geode-core/src/main/java/org/apache/geode/internal/cache/BucketRedundancyTracker.java --- @@ -0,0 +1,108 @@ +/* + * 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; + +/** + * Keeps track of redundancy status for a PartitionedRegion and (if enabled) updates the statistics --- End diff -- did you mean to say BucketRegion instead of PartitionedRegion? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #559: GEODE-1672 When amount of overflowed persisted data...
Github user dschneider-pivotal commented on a diff in the pull request: https://github.com/apache/geode/pull/559#discussion_r120224399 --- Diff: geode-docs/managing/troubleshooting/system_failure_and_recovery.html.md.erb --- @@ -276,8 +276,83 @@ find the reason. Description: -The process discovered that it was not in the distributed system and cannot determine why it was removed. The membership coordinator removed the member after it failed to respond to an internal are you alive message. +The process discovered that it was not in the distributed system and cannot determine why it was +removed. The membership coordinator removed the member after it failed to respond to an internal +are-you-alive message. Response: The operator should examine the locator processes and logs. + +## Restart Fails Due To Out-of-Memory Error + +This section describes a restart failure that can occur when the stopped system is one that was configured with persistent regions. Specifically: + +- Some of the regions of the recovering system, when running, were configured as PERSISTENT regions, which means that they save their data to disk. +- At least one of the persistent regions was configured to evict least recently used (LRU) data by overflowing values to disk. + +### How Data is Recovered From Persistent Regions + +Data recovery, upon restart, always recovers keys. You can configure whether and how the system +recovers the values associated with those keys to populate the system cache. + +**Value Recovery** + +- Recovering all values immediately during startup slows the startup time but results in consistent +read performance after the startup on a "hot" cache. + +- Recovering no values means quicker startup but a "cold" cache, so the first retrieval of each value will read from disk. + +- Retrieving values asynchronously in a background thread allows a relatively quick startup on a "warm" cache +that will eventually recover every value. + +**Retrieve or Ignore LRU values** + +When a system with persistent LRU regions shuts down, the system does not record which of the values +were recently used. On subsequent startup, if values are recovered into an LRU region they may be +the least recently used instead of the most recently used. Also, if LRU values are recovered on a +heap or an off-heap LRU region, it is possible that the LRU memory limit will be exceeded, resulting +in an `OutOfMemoryException` during recovery. For these reasons, LRU value recovery can be treated +differently than non-LRU values. + +## Default Recovery Behavior for Persistent Regions + +The default behavior is for the system to recover all keys, then asynchronously recover all data +values that were resident, leaving LRU values unrecovered. This default strategy is best for +most applications, because it strikes a balance between recovery speed and cache completeness. + +### Configuring Recovery of Persistent Regions + +Three Java system parameters allow the developer to control the recovery behavior for persistent regions: + +- `gemfire.disk.recoverValues` + + Default = `true`, recover values. If `false`, recover only keys, do not recover values. + + *How used:* When `true`, recovery of the values "warms up" the cache so data retrievals will find + their values in the cache, without causing time consuming disk accesses. When `false`, shortens + recovery time so the system becomes available for use sooner, but the first retrieval on each key + will require a disk read. + +- `gemfire.disk.recoverLruValues` + + Default = `false`, do not recover LRU values. If `true`, recover LRU values. If + `gemfire.disk.recoverValues` is `false`, then `gemfire.disk.recoverLruValues` is ignored, since + no values are recovered. + + *How used:* When `false`, shortens recovery time by ignoring LRU values. When `true`, restores + more data values to the cache. Recovery of the LRU values increases heap memory usage and + could cause an out-of-memory error, preventing the system from restarting. + +- `gemfire.disk.recoverValuesSync` + + Default = `false`, recover values by an asynchronous background process. If `true`, values are + recovered synchronously, and recovery is not complete until all values have been retrieved. If + `gemfire.disk.recoverValues` is `false`, then `gemfire.disk.recoverValuesSync` is ignored since + no values are recovered. + + *How used:* When `false`, allows the system to become available sooner, but some time must elapse + before the entire cache is refreshed. Some key retrievals will require disk acc
[GitHub] geode pull request #559: GEODE-1672 When amount of overflowed persisted data...
Github user dschneider-pivotal commented on a diff in the pull request: https://github.com/apache/geode/pull/559#discussion_r120222821 --- Diff: geode-docs/managing/troubleshooting/system_failure_and_recovery.html.md.erb --- @@ -276,8 +276,83 @@ find the reason. Description: -The process discovered that it was not in the distributed system and cannot determine why it was removed. The membership coordinator removed the member after it failed to respond to an internal are you alive message. +The process discovered that it was not in the distributed system and cannot determine why it was +removed. The membership coordinator removed the member after it failed to respond to an internal +are-you-alive message. Response: The operator should examine the locator processes and logs. + +## Restart Fails Due To Out-of-Memory Error + +This section describes a restart failure that can occur when the stopped system is one that was configured with persistent regions. Specifically: + +- Some of the regions of the recovering system, when running, were configured as PERSISTENT regions, which means that they save their data to disk. +- At least one of the persistent regions was configured to evict least recently used (LRU) data by overflowing values to disk. + +### How Data is Recovered From Persistent Regions + +Data recovery, upon restart, always recovers keys. You can configure whether and how the system +recovers the values associated with those keys to populate the system cache. + +**Value Recovery** + +- Recovering all values immediately during startup slows the startup time but results in consistent +read performance after the startup on a "hot" cache. + +- Recovering no values means quicker startup but a "cold" cache, so the first retrieval of each value will read from disk. + +- Retrieving values asynchronously in a background thread allows a relatively quick startup on a "warm" cache +that will eventually recover every value. + +**Retrieve or Ignore LRU values** + +When a system with persistent LRU regions shuts down, the system does not record which of the values +were recently used. On subsequent startup, if values are recovered into an LRU region they may be +the least recently used instead of the most recently used. Also, if LRU values are recovered on a +heap or an off-heap LRU region, it is possible that the LRU memory limit will be exceeded, resulting +in an `OutOfMemoryException` during recovery. For these reasons, LRU value recovery can be treated +differently than non-LRU values. + +## Default Recovery Behavior for Persistent Regions + +The default behavior is for the system to recover all keys, then asynchronously recover all data +values that were resident, leaving LRU values unrecovered. This default strategy is best for --- End diff -- drop "that were resident" --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #525: GEODE-2962: Add more messages for compact disk-store
Github user dschneider-pivotal commented on the issue: https://github.com/apache/geode/pull/525 Your diffs now show 51 files changed. It now shows a removal of deprecated FunctionService API. Perhaps you are picking this up from changes others made? You might need to close this PR and start a new one but can you get it back only showing the changes you are making? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #507: GEODE-231 : Remove deprecated AttributesMutator.setCacheLi...
Github user dschneider-pivotal commented on the issue: https://github.com/apache/geode/pull/507 +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #507: GEODE-231 : Remove deprecated AttributesMutator.set...
Github user dschneider-pivotal commented on a diff in the pull request: https://github.com/apache/geode/pull/507#discussion_r118060767 --- Diff: geode-junit/src/main/java/org/apache/geode/test/junit/rules/UseJacksonForJsonPathRule.java --- @@ -14,19 +14,18 @@ */ package org.apache.geode.test.junit.rules; -import java.util.EnumSet; -import java.util.Set; --- End diff -- I see no reason why this import reorg should be part of this pull request. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #515: GEODE-240: Remove deprecated methods on TransactionEvent
Github user dschneider-pivotal commented on the issue: https://github.com/apache/geode/pull/515 This looks good --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #525: GEODE-2962: Add more messages for compact disk-stor...
Github user dschneider-pivotal commented on a diff in the pull request:
https://github.com/apache/geode/pull/525#discussion_r118057642
--- Diff:
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
---
@@ -485,9 +485,14 @@ public Result compactDiskStore(
memberCompactInfo.clear();
}
String notExecutedMembers =
CompactRequest.getNotExecutedMembers();
+if (notExecutedMembers != null &&
!notExecutedMembers.isEmpty()) {
+ notExecutedMembers = "but was not send to " +
notExecutedMembers;
+} else {
+ notExecutedMembers = "all the members";
--- End diff --
I think this log message only needs to say something if
"notExecutedMembers" is not null and not empty.
No need for a message in the else. You will notice that immediately after
this code is a report of what was compacted so it seem to me too verbose to say
"I sent the request to everyone".
But it could be helpful to log a note of the members that did not receive
the request.
So I think you should put the log lines 493-496 inside this if and get rid
of the else.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #504: GEODE-254: Removed deprecated Region.keys and Region.entri...
Github user dschneider-pivotal commented on the issue: https://github.com/apache/geode/pull/504 Yes, pull it in --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #504: GEODE-254: Removed deprecated Region.keys and Region.entri...
Github user dschneider-pivotal commented on the issue: https://github.com/apache/geode/pull/504 +1 LGTM --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #495: GEODE-234: Removed deprecated MirrorType class and its usa...
Github user dschneider-pivotal commented on the issue: https://github.com/apache/geode/pull/495 Yes. I think it is just xsd (not dtd) changes and the java changes needed when adding a new xsd. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #488: GEODE-254: Removed deprecated Region.keys and Regio...
Github user dschneider-pivotal commented on a diff in the pull request:
https://github.com/apache/geode/pull/488#discussion_r115331529
--- Diff:
geode-core/src/main/java/org/apache/geode/cache/client/internal/ProxyRegion.java
---
@@ -388,7 +388,7 @@ public Set keySetOnServer() {
public Set keys() {
--- End diff --
I think you want this old "keys()" implementation to now be "keySet()". The
old "keySet" did not properly call preOp/postOp.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #495: GEODE-234: Removed deprecated MirrorType class and its usa...
Github user dschneider-pivotal commented on the issue: https://github.com/apache/geode/pull/495 The xsd you are looking for is: geode-core/build/resources/main/META-INF/schemas/geode.apache.org/schema/cache/cache-1.0.xsd However you should not modify this one since it has already been released. The next geode release will be 1.2 so you would want to create a new cache-1.2.xsd and remove mirror-type from it. Introducing a new xsd involves a bunch of busy work. You need to add org.apache.geode.internal.cache.xmlcache.CacheXml.VERSION_1_2, update CacheXml.VERSION_LATEST, add CacheXml.SCHEMA_1_2_LOCATION, add CacheXmlVersion.GEODE_1_2, add CacheXmlGeode12DUnitTest that extends CacheXmlGeode10DUnitTest. I'm not sure what should be done about these dunit tests. They test the old xsd/dtds which still do support mirror-type. If we remove support for the mirror-type attribute from the xml parser code then all these old dtds and xsds will no longer support mirror-type in their xml even though the old dtd/xsd says it does. I think all the old dtds we still support also have data-policy so even with that old dtd you could convert the mirror-type attribute to data-policy in you existing xml. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #443: [GEODE-1887] #comment Fix for Client PROXY region should d...
Github user dschneider-pivotal commented on the issue: https://github.com/apache/geode/pull/443 I think this pull request should be closed. GEODE-1887 will be updated with a plan for doing it without breaking backwards compatibility. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #464: GEODE-237: Removed deprecated API from EntryEvent
Github user dschneider-pivotal commented on the issue: https://github.com/apache/geode/pull/464 This change looks good. I noticed travis CI failed with the following. But it looks like isExpiration was removed in some other commit so it is not clear to me why this one got the following failure. :geode-core:compileTestJava/home/travis/build/apache/geode/geode-core/src/test/java/org/apache/geode/TXJUnitTest.java:3009: error: cannot find symbol assertTrue(!event.isExpiration()); ^ symbol: method isExpiration() location: variable event of type EntryEvent --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #488: GEODE-254: Removed deprecated Region.keys and Region.entri...
Github user dschneider-pivotal commented on the issue: https://github.com/apache/geode/pull/488 This looks good. Pull it in. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #440: GEODE-2723: Removed localCacheEnabled field, and associate...
Github user dschneider-pivotal commented on the issue: https://github.com/apache/geode/pull/440 Looks good to merge --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #423: Feature/geode 1195
Github user dschneider-pivotal commented on the issue: https://github.com/apache/geode/pull/423 Looks good --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #320: GEODE-1969 : oplog closed while writing to oplog with gemf...
Github user dschneider-pivotal commented on the issue: https://github.com/apache/geode/pull/320 After looking over this fix we decided that it is too simple to have a reasonable unit test. I will pull it in --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #320: GEODE-1969 : oplog closed while writing to oplog with gemf...
Github user dschneider-pivotal commented on the issue: https://github.com/apache/geode/pull/320 We need a unit test for GEODE-1969 that reproduces that failure. Then apply your fix and the unit test should pass. The unit test should be made part of this pull request. Hope fully Oplog will allow you to mock the channel. You might need to do some refactoring of Oplog to make it friendly to mocking a unit test. I think for this ticket most of the work will be in the unit test. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
