[GitHub] [geode] jdeppe-pivotal commented on pull request #3927: GEODE-6805: Allow shorthand ~ to represent user home in jar deployment.

2019-08-15 Thread GitHub
Technically (at least considering shell semantics) `~foo` is legitimate (it expands to the home directory of the `foo` user if that user exists) but I don't think we should attempt to expand that. [ Full content available at: https://github.com/apache/geode/pull/3927 ] This message was relayed

[GitHub] [geode] dschneider-pivotal commented on pull request #3928: GEODE-6945:geode-managment should create its own set of configuration…

2019-08-15 Thread GitHub
add unit test coverage for this method to RegionConfigRealizer. It would be good for that method to verify that this method does the right thing for every RegionType and that the attributes we support make it to the RegionFactory. [ Full content available at:

[GitHub] [geode] bschuchardt closed pull request #3908: GEODE-7072 CI Failure: WANRollingUpgradeEventProcessingMixedSiteOneCurrentSiteTwo

2019-08-15 Thread GitHub
[ pull request closed by bschuchardt ] [ Full content available at: https://github.com/apache/geode/pull/3908 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] mivanac commented on pull request #510: GEODE-7086: coding impacts for support of IPv6

2019-08-15 Thread GitHub
This is modified, because for IPV6, where ever method get_ip_address() is called, error is reported, and get_addr() must be used. I added this check, to verify is hostname resolved. [ Full content available at: https://github.com/apache/geode-native/pull/510 ] This message was relayed via

[GitHub] [geode] lgtm-com[bot] commented on issue #3928: GEODE-6945:geode-managment should create its own set of configuration…

2019-08-15 Thread lgtm-com
This pull request **introduces 1 alert** and **fixes 1** when merging 51e25b17f55d107901c434449e180ab17a69c24d into 6f4bbbd96bcecdb82cf7753ce1dae9fa6baebf9b - [view on LGTM.com](https://lgtm.com/projects/g/apache/geode/rev/pr-5b70645175cd713c75738f8564fea692ad6ee9b9) **new alerts:** * 1 for

[GitHub] [geode] upthewaterspout closed pull request #3922: GEODE-7085: Ensuring the bitset stays within BIT_SET_WIDTH

2019-08-15 Thread GitHub
[ pull request closed by upthewaterspout ] [ Full content available at: https://github.com/apache/geode/pull/3922 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jujoramos closed pull request #3897: GEODE-7062: Fix Race Condition in DUnit Test

2019-08-15 Thread GitHub
[ pull request closed by jujoramos ] [ Full content available at: https://github.com/apache/geode/pull/3897 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode]

2019-08-15 Thread GitHub
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #510: GEODE-7086: coding impacts for support of IPv6

2019-08-15 Thread GitHub
Yup, it's needed by the protocol / message here unfortunately. [ Full content available at: https://github.com/apache/geode-native/pull/510 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #510: GEODE-7086: coding impacts for support of IPv6

2019-08-15 Thread GitHub
This is where I would just put the host adder into the vector so there is no question of ownership later. [ Full content available at: https://github.com/apache/geode-native/pull/510 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #510: GEODE-7086: coding impacts for support of IPv6

2019-08-15 Thread GitHub
Why not make the caller send this object the vector? Then the ambiguity of the `hostAddrLocalMem` check goes away. The owner is always the caller making the conversion to a vector. [ Full content available at: https://github.com/apache/geode-native/pull/510 ] This message was relayed via

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #510: GEODE-7086: coding impacts for support of IPv6

2019-08-15 Thread GitHub
Comment not valid anymore. [ Full content available at: https://github.com/apache/geode-native/pull/510 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #510: GEODE-7086: coding impacts for support of IPv6

2019-08-15 Thread GitHub
Can we create a common method so we don't have duplicate IPv4 and IPv6 code in multiple places? [ Full content available at: https://github.com/apache/geode-native/pull/510 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #510: GEODE-7086: coding impacts for support of IPv6

2019-08-15 Thread GitHub
Perhaps add a `void writeBytes(const std::vector& bytes)` method to DataOutput? [ Full content available at: https://github.com/apache/geode-native/pull/510 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #510: GEODE-7086: coding impacts for support of IPv6

2019-08-15 Thread GitHub
Doesn't ACE has some encapsulation for host addr, not that I won't to increase our use of ACE but better that then our own invention right now? [ Full content available at: https://github.com/apache/geode-native/pull/510 ] This message was relayed via gitbox.apache.org for

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #510: GEODE-7086: coding impacts for support of IPv6

2019-08-15 Thread GitHub
Remove commented out code. [ Full content available at: https://github.com/apache/geode-native/pull/510 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] balesh2 opened pull request #3931: GEODE-7054: add tests running with SSL

2019-08-15 Thread GitHub
* add tasks that run benchmark with SSL * pass option to run benchmark script Signed-off-by: Murtuza Boxwala * create cluster for SSL with a different cluster tag * filter the tests run with SSL * run the SSL and non-SSL tasks in parallel Thank you for submitting a contribution to Apache

[GitHub] [geode] lgtm-com[bot] commented on issue #3914: DRAFT: GEODE-6964: Move geode log4j core classes to geode-log4j

2019-08-15 Thread lgtm-com
This pull request **introduces 3 alerts** and **fixes 4** when merging 5370329c81dcddd26ecfff3d68583d6e768b82fa into 86fd74db98b5dff0e92ea4985651f3955c1a3420 - [view on LGTM.com](https://lgtm.com/projects/g/apache/geode/rev/pr-d87bd3bb50d8ac1ee1c63bf986d3ff0b6b44e505) **new alerts:** * 3 for

[GitHub] [geode] mhansonp commented on issue #3935: GEODE-7091: Add Micrometer binders to meter registry

2019-08-15 Thread GitHub
My bad. :( Thanks, Mark [ Full content available at: https://github.com/apache/geode/pull/3935 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] kirklund commented on issue #3904: GEODE-7070: Remove AlertAppender usage from GMSMembershipManager

2019-08-15 Thread GitHub
@bschuchardt (Not sure if my editing made this more readable or not) I'm actually a little worried about merging this change in. Without this change, LynnG encountered a failure which I think might occur more if we merge this in. LynnG's analysis: In this run we are expecting

[GitHub] [geode] mhansonp closed pull request #3935: GEODE-7091: Add Micrometer binders to meter registry

2019-08-15 Thread GitHub
[ pull request closed by mhansonp ] [ Full content available at: https://github.com/apache/geode/pull/3935 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] kamilla1201 commented on pull request #3931: GEODE-7054: add tests running with SSL

2019-08-15 Thread GitHub
You can also filter Put and Get benchmarks this way: --tests *GetBenchmark --tests *PutBenchmark [ Full content available at: https://github.com/apache/geode/pull/3931 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] mhansonp closed pull request #3915: GEODE-7082: geode-core:distributedTest timeout exceeded.

2019-08-15 Thread GitHub
[ pull request closed by mhansonp ] [ Full content available at: https://github.com/apache/geode/pull/3915 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jinmeiliao opened pull request #3936: GEODE-6945: no need to expand the refid attributes in the xml

2019-08-15 Thread GitHub
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

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #510: GEODE-7086: coding impacts for support of IPv6

2019-08-15 Thread GitHub
I think just checking that `get_addr()` is not `nullptr` should suffice. [ Full content available at: https://github.com/apache/geode-native/pull/510 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] mhansonp commented on issue #3935: GEODE-7091: Add Micrometer binders to meter registry

2019-08-15 Thread GitHub
My bad. :( Thanks, Mark > On Aug 15, 2019, at 2:56 PM, M. Oleske wrote: > > It may have been good to let the acceptance test and distributed test finish > (especially since there was a new acceptance test) > > — > You are receiving this because you modified the open/close state. > Reply to

[GitHub] [geode] kirklund commented on issue #3904: GEODE-7070: Remove AlertAppender usage from GMSMembershipManager

2019-08-15 Thread GitHub
@bschuchardt I'm actually a little worried about merging this change in. Without this change, LynnG encountered a failure which I think might occur more if we merge this in. In this run we are expecting ForcedDisconnects, but they don't occur until after the hang is declared. The difference

[GitHub] [geode] kirklund commented on issue #3904: GEODE-7070: Remove AlertAppender usage from GMSMembershipManager

2019-08-15 Thread GitHub
@bschuchardt I'm actually a little worried about merging this change in. Without this change, LynnG encountered a failure which I think might occur more if we merge this in. In this run we are expecting ForcedDisconnects, but they don't occur until after the hang is declared. The difference

[GitHub] [geode] mcmellawatt commented on pull request #3922: GEODE-7085: Ensuring the bitset stays within BIT_SET_WIDTH

2019-08-15 Thread GitHub
Sorry for dumb questions, but is this cast safe? Is it possible newVersion - bitSetVersion could be > Integer.MAX_VALUE e.g. if newVersion were Long.MAX_VALUE and bitSetVersion were 1, or do we know contextually that is not possible here? [ Full content available at:

[GitHub] [geode] mcmellawatt commented on pull request #3922: GEODE-7085: Ensuring the bitset stays within BIT_SET_WIDTH

2019-08-15 Thread GitHub
typo `maxmimumClearBit` [ Full content available at: https://github.com/apache/geode/pull/3922 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] mcmellawatt commented on pull request #3922: GEODE-7085: Ensuring the bitset stays within BIT_SET_WIDTH

2019-08-15 Thread GitHub
larger [ Full content available at: https://github.com/apache/geode/pull/3922 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jujoramos commented on pull request #3932: GEODE 6872: Add Non-Sticky configuration information to Tomcat Session module documentation

2019-08-15 Thread GitHub
Same as before: we need to remove the ` System.out.println` call. [ Full content available at: https://github.com/apache/geode/pull/3932 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jujoramos commented on pull request #3932: GEODE 6872: Add Non-Sticky configuration information to Tomcat Session module documentation

2019-08-15 Thread GitHub
Same as before: we need to remove the ` System.out.println` call. [ Full content available at: https://github.com/apache/geode/pull/3932 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jujoramos commented on pull request #3932: GEODE 6872: Add Non-Sticky configuration information to Tomcat Session module documentation

2019-08-15 Thread GitHub
Same as before: we need to remove the ` System.out.println` call. [ Full content available at: https://github.com/apache/geode/pull/3932 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jujoramos commented on pull request #3932: GEODE 6872: Add Non-Sticky configuration information to Tomcat Session module documentation

2019-08-15 Thread GitHub
Same as before: we need to remove the ` System.out.println` call. [ Full content available at: https://github.com/apache/geode/pull/3932 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #510: GEODE-7086: coding impacts for support of IPv6

2019-08-15 Thread GitHub
It's an option to try. Both are better than duplicated logic. [ Full content available at: https://github.com/apache/geode-native/pull/510 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] upthewaterspout commented on pull request #3922: GEODE-7085: Ensuring the bitset stays within BIT_SET_WIDTH

2019-08-15 Thread GitHub
Well, this is in the else clause - we we know that newVersion <= bitSetVersion+bitSet.size(). But maybe it would have been better to use Math.toIntExact here to validate that. [ Full content available at: https://github.com/apache/geode/pull/3922 ] This message was relayed via gitbox.apache.org

[GitHub] [geode] BenjaminPerryRoss commented on pull request #3932: GEODE 6872: Add Non-Sticky configuration information to Tomcat Session module documentation

2019-08-15 Thread GitHub
Not sure how those made their way into these commits, I'll remove them [ Full content available at: https://github.com/apache/geode/pull/3932 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] mcmellawatt opened pull request #3933: GEODE-7090: Copy message to isolate filter info calculation

2019-08-15 Thread GitHub
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

[GitHub] [geode] aaronlindsey opened pull request #3935: GEODE-7091: Add Micrometer binders to meter registry

2019-08-15 Thread GitHub
Add the following Micrometer binders: - JvmGcMetrics - ProcessorMetrics - JvmThreadMetrics - UptimeMetrics - FileDescriptorMetrics Co-authored-by: Aaron Lindsey Co-authored-by: Kirk Lund Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the

[GitHub] [geode] pivotal-eshu opened pull request #3934: GEODE-6613: Do not set TransactionTimeout in the test.

2019-08-15 Thread GitHub
[ Full content available at: https://github.com/apache/geode/pull/3934 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] BenjaminPerryRoss opened pull request #3932: GEODE 6872: Add Non-Sticky configuration information to Tomcat Session module documentation

2019-08-15 Thread GitHub
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

[GitHub] [geode] aaronlindsey commented on issue #3938: GEODE-7091: Add Micrometer binders to meter registry

2019-08-15 Thread GitHub
This is the same PR as #3935, except that the FileDescriptorMetrics binder tests are ignored on Windows. @kirklund @demery-pivotal @mhansonp [ Full content available at: https://github.com/apache/geode/pull/3938 ] This message was relayed via gitbox.apache.org for

[GitHub] [geode] aaronlindsey opened pull request #3938: GEODE-7091: Add Micrometer binders to meter registry

2019-08-15 Thread GitHub
Add the following Micrometer binders: - JvmGcMetrics - ProcessorMetrics - JvmThreadMetrics - UptimeMetrics - FileDescriptorMetrics Ignore FileDescriptorMetrics binder tests on Windows Co-authored-by: Aaron Lindsey Co-authored-by: Kirk Lund Thank you for submitting a contribution to Apache

[GitHub] [geode] mcmellawatt commented on issue #3886: GEODE-6867: Updating Tomcat Session State Module docs

2019-08-15 Thread GitHub
> Descriptive text looks fine. Alerts about updating the corresponding user > guide passages may be helpful, but one wonders if they're truly necessary. > Shouldn't developers _always_ check behavioral changes against the > documentation? Never mind, silly question :) I agree in an ideal world