[GitHub] nabarunnag opened a new pull request #16: GEODE-6084: Refactored the benchmark code as per review.
nabarunnag opened a new pull request #16: GEODE-6084: Refactored the benchmark code as per review. URL: https://github.com/apache/geode-benchmarks/pull/16 This is an automated message from the Apache Git Service. To respond to the message, please log on 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 With regards, Apache Git Services
[GitHub] [geode-benchmarks] nabarunnag opened pull request #16: GEODE-6084: Refactored the benchmark code as per review.
[ Full content available at: https://github.com/apache/geode-benchmarks/pull/16 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] agingade commented on pull request #2950: GEODE-6142: Check JDBC mapping before destroy region
What happens when region is destroyed with API and it has jdbc-mapping? [ Full content available at: https://github.com/apache/geode/pull/2950 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] PurelyApplied closed pull request #2939: GEODE-5883: Add Nebula dependency linter as gradle utility
[ pull request closed by PurelyApplied ] [ Full content available at: https://github.com/apache/geode/pull/2939 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] PurelyApplied closed pull request #2920: GEODE-6100: Cleanup suspect string logic for better readability
[ pull request closed by PurelyApplied ] [ Full content available at: https://github.com/apache/geode/pull/2920 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] dschneider-pivotal commented on pull request #2950: GEODE-6142: Check JDBC mapping before destroy region
Because the only public way we have of destroying a jdbc-mapping is a gfsh command. All those other examples also have public apis that let you remove the other thing. Although at least some of your examples do not prevent a region from being destroyed (cache writer and loader; I don't know about cqs and indexes). The goal here is to help the gfsh user but not the api user. [ Full content available at: https://github.com/apache/geode/pull/2950 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] BenjaminPerryRoss commented on pull request #2954: GEODE-6066: Fixed cluster config update for error case
I updated this to use the term configuration rather than the abbreviation. [ Full content available at: https://github.com/apache/geode/pull/2954 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] jxblum commented on issue #2953: GEODE-6032: Updated javadoc to clarify what is communicated with hasD…
See my [recent comment](https://issues.apache.org/jira/browse/GEODE-6032?focusedCommentId=16710787=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16710787) on GEODE-6032. [ Full content available at: https://github.com/apache/geode/pull/2953 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] WireBaron opened pull request #2958: GEODE-5674: Stop using random values for test ports
Rebased to current develop and no longer try to get keeper objects for UDP ports. Also added better testing of the new behavior. 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: - [x] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [x] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [x] Does `gradlew build` run cleanly? - [x] 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, you check travis-ci 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. [ Full content available at: https://github.com/apache/geode/pull/2958 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] dschneider-pivotal commented on issue #2957: GEODE-6151: use same term for JDBC mapping
@BenjaminPerryRoss @monkeyherder @gemzdude please review [ Full content available at: https://github.com/apache/geode/pull/2957 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] dschneider-pivotal opened pull request #2957: GEODE-6151: use same term for JDBC mapping
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, you check travis-ci 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. [ Full content available at: https://github.com/apache/geode/pull/2957 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-benchmarks] smgoller closed pull request #15: Convert to sole tenant style cluster.
[ pull request closed by smgoller ] [ Full content available at: https://github.com/apache/geode-benchmarks/pull/15 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] smgoller closed pull request #15: Convert to sole tenant style cluster.
smgoller closed pull request #15: Convert to sole tenant style cluster. URL: https://github.com/apache/geode-benchmarks/pull/15 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/infrastructure/google_cloud/destroy_cluster.sh b/infrastructure/google_cloud/destroy_cluster.sh index 828264b..7247107 100755 --- a/infrastructure/google_cloud/destroy_cluster.sh +++ b/infrastructure/google_cloud/destroy_cluster.sh @@ -19,6 +19,10 @@ TAG=${1} PREFIX="geode-performance-${TAG}" +NODE_TEMPLATE="${PREFIX}-node-template" +NODE_GROUP="${PREFIX}-node-group" gcloud compute instance-groups managed delete ${PREFIX} --quiet gcloud compute instance-templates delete ${PREFIX}-template --quiet +gcloud compute sole-tenancy node-groups delete ${NODE_GROUP} +gcloud compute sole-tenancy node-templates delete ${NODE_TEMPLATE} \ No newline at end of file diff --git a/infrastructure/google_cloud/launch_cluster.sh b/infrastructure/google_cloud/launch_cluster.sh index f720ad8..51be8a0 100755 --- a/infrastructure/google_cloud/launch_cluster.sh +++ b/infrastructure/google_cloud/launch_cluster.sh @@ -24,16 +24,21 @@ COUNT=${2} SUBNET=${3} IMAGE_FAMILY="geode-performance" PREFIX="geode-performance-${TAG}" -INSTANCE_TYPE=n1-standard-16 +NODE_TEMPLATE="${PREFIX}-node-template" +NODE_GROUP="${PREFIX}-node-group" +INSTANCE_TYPE=n1-highmem-96 KEY_FILE=/tmp/id_${TAG} ssh-keygen -b 2048 -t rsa -f $KEY_FILE -q -N "" +gcloud compute sole-tenancy node-templates create ${NODE_TEMPLATE} --node-type=n1-node-96-624 +gcloud compute sole-tenancy node-groups create ${NODE_GROUP} --node-template=${NODE_TEMPLATE} --target-size=${COUNT} gcloud compute instance-templates create ${PREFIX}-template \ --machine-type=${INSTANCE_TYPE} \ --subnet=${SUBNET} \ + --node-group="${NODE_GROUP}" \ --image-family=${IMAGE_FAMILY} \ --boot-disk-size=50GB \ --boot-disk-type=pd-ssd This is an automated message from the Apache Git Service. To respond to the message, please log on 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 With regards, Apache Git Services
[GitHub] [geode] PurelyApplied commented on issue #2939: GEODE-5883: Add Nebula dependency linter as gradle utility
Failure at `291134b` appears to be open ticket GEODE-6008. Rerunning precheckin as part of this merge in any case. [ Full content available at: https://github.com/apache/geode/pull/2939 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] jinmeiliao closed pull request #2951: GEODE-5971: JaxbService should be able to unmarshall older namespace xml
[ pull request closed by jinmeiliao ] [ Full content available at: https://github.com/apache/geode/pull/2951 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] kirklund commented on pull request #2953: GEODE-6032: Updated javadoc to clarify what is communicated with hasD…
Here's my version: _Returns true if this object has pending changes that can be transmitted as a delta._ If that's not enough then maybe: _Returns true if this object has pending changes that can be transmitted as a delta. Returns false if this object must be transmitted in its entirety._ [ Full content available at: https://github.com/apache/geode/pull/2953 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] kirklund commented on pull request #2953: GEODE-6032: Updated javadoc to clarify what is communicated with hasD…
Here's my version: ``` Returns true if this object has pending changes that can be transmitted as a delta. ``` If that's not enough then maybe: ``` Returns true if this object has pending changes that can be transmitted as a delta. Returns false if this object must be transmitted in its entirety. ``` [ Full content available at: https://github.com/apache/geode/pull/2953 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] smgoller opened a new pull request #15: Convert to sole tenant style cluster.
smgoller opened a new pull request #15: Convert to sole tenant style cluster. URL: https://github.com/apache/geode-benchmarks/pull/15 This launches the cluster using single-tenant nodes instead of random instances. This is an automated message from the Apache Git Service. To respond to the message, please log on 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 With regards, Apache Git Services
[GitHub] [geode-benchmarks] smgoller opened pull request #15: Convert to sole tenant style cluster.
This launches the cluster using single-tenant nodes instead of random instances. [ Full content available at: https://github.com/apache/geode-benchmarks/pull/15 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] bschuchardt opened pull request #2956: GEODE-2113 Implement SSL over NIO
This removes old-I/O use in TCPConduit peer-to-peer communications. This was used for SSL/TLS secure commuications but Java has had an SSLEngine implementation that allows you to implement secure communications on new-I/O SocketChannels or any other transport mechanism. A new NioSSLEngine class wraps the JDK's SSLEngine and provides the SSL handshake as well as encryption/decryption of messages. SocketCreator initiates the SSL handshake and returns a NioSslEngine that TCPConduit then uses for messaging. I've also done a lot of cleanup of compilation warnings in Connection.java and removed references to "NIO". The primary SSL/TLS changes in that class are in writeFully (renamed from nioWriteFully) and processBuffer (renamed from processNIOBuffer). Porting client/server to use NioSSLEngine will be done under a separate ticket and a different version of NioEngine may be created to secure UDP messaging. 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: - [x] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [x] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [x] Is your initial contribution a single, squashed commit? - [x] Does `gradlew build` run cleanly? - [x] 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, you check travis-ci 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. [ Full content available at: https://github.com/apache/geode/pull/2956 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] dschneider-pivotal commented on pull request #2954: GEODE-6066: Fixed cluster config update for error case
Oops, I just saw we do check "service != null. So I'd be satisfied if you just change "config" to "configuration". [ Full content available at: https://github.com/apache/geode/pull/2954 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] dschneider-pivotal commented on pull request #2954: GEODE-6066: Fixed cluster config update for error case
The problem I have with saying "data source saved to cluster config" is it might not happen. I suggest only adding this to the message here if "service" is not null. Even in that case should we say here: "will attempt to save to cluster configuration" instead? (Note: change "config" to "configuration" in customer facing message). We will ask gfsh to make the attempt and it will follow it up with its own message telling us if it was saved to cluster config. It is not until "updateConfigForGroup" is called and returns true that we know the cluster config was actually saved. [ Full content available at: https://github.com/apache/geode/pull/2954 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] agingade commented on pull request #2953: GEODE-6032: Updated javadoc to clarify what is communicated with hasD…
Thanks Bruce. I wanted to make it clear what is communicated based on hasDelta(). [ Full content available at: https://github.com/apache/geode/pull/2953 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] aditya87 commented on pull request #2951: GEODE-5971: JaxbService should be able to unmarshall older namespace xml
I wonder if we should just replace this regardless of the original URI [ Full content available at: https://github.com/apache/geode/pull/2951 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pdxcodemonkey opened pull request #414: GEODE-6139: Enforce Apache Rat findings in Travis CI
[ Full content available at: https://github.com/apache/geode-native/pull/414 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] mcmellawatt opened pull request #2955: GEODE-6143: Removing PowerMock from BackupFileCopierIntegrationTest
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: - [X] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [X] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [X] Is your initial contribution a single, squashed commit? - [X] Does `gradlew build` run cleanly? - [X] Have you written or updated unit tests to verify your changes? - N/A 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, you check travis-ci 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. [ Full content available at: https://github.com/apache/geode/pull/2955 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] bschuchardt commented on pull request #2953: GEODE-6032: Updated javadoc to clarify what is communicated with hasD…
I'm sorry but this javadoc doesn't seem at all clear to me and it has grammatical errors. I think you should keep the original Javadoc and append something like this: _Returns false if pending changes are not available, which will cause the entire object to be transmitted to other processes._ [ Full content available at: https://github.com/apache/geode/pull/2953 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] dhemery closed pull request #2788: GEODE-5994: Add CPUContentionService
[ pull request closed by dhemery ] [ Full content available at: https://github.com/apache/geode/pull/2788 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] gesterzhou opened pull request #2952: GEODE-6149: when client's cache is closing, its GetClientPRMetaDataOp…
… could end up with NPE @jhuynh1 @bschuchardt 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, you check travis-ci 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. [ Full content available at: https://github.com/apache/geode/pull/2952 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] upthewaterspout closed pull request #13: Ignoring extra hosts in SshInfrastructure
upthewaterspout closed pull request #13: Ignoring extra hosts in SshInfrastructure URL: https://github.com/apache/geode-benchmarks/pull/13 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/harness/src/main/java/org/apache/geode/perftest/infrastructure/ssh/SshInfrastructureFactory.java b/harness/src/main/java/org/apache/geode/perftest/infrastructure/ssh/SshInfrastructureFactory.java index 00ed0c5..f63cda2 100644 --- a/harness/src/main/java/org/apache/geode/perftest/infrastructure/ssh/SshInfrastructureFactory.java +++ b/harness/src/main/java/org/apache/geode/perftest/infrastructure/ssh/SshInfrastructureFactory.java @@ -19,13 +19,14 @@ import java.util.Arrays; import java.util.Collection; +import java.util.List; import org.apache.geode.perftest.infrastructure.Infrastructure; import org.apache.geode.perftest.infrastructure.InfrastructureFactory; public class SshInfrastructureFactory implements InfrastructureFactory { - private final Collection hosts; + private final List hosts; private final String user; public SshInfrastructureFactory(String user, String... hosts) { @@ -34,8 +35,12 @@ public SshInfrastructureFactory(String user, String... hosts) { } @Override - public Infrastructure create(int nodes) throws Exception { -return new SshInfrastructure(hosts, user); + public Infrastructure create(int nodes) { +if (nodes > hosts.size()) { + throw new IllegalStateException( + "Not enough hosts to create " + nodes + " nodes. Available hosts: " + hosts); +} +return new SshInfrastructure(hosts.subList(0, nodes), user); } public Collection getHosts() { diff --git a/harness/src/test/java/org/apache/geode/perftest/infrastructure/ssh/SshInfrastructureFactoryTest.java b/harness/src/test/java/org/apache/geode/perftest/infrastructure/ssh/SshInfrastructureFactoryTest.java new file mode 100644 index 000..9c929a1 --- /dev/null +++ b/harness/src/test/java/org/apache/geode/perftest/infrastructure/ssh/SshInfrastructureFactoryTest.java @@ -0,0 +1,36 @@ +/* + * 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.perftest.infrastructure.ssh; + +import org.assertj.core.api.Assertions; +import org.junit.Test; + +import org.apache.geode.perftest.infrastructure.Infrastructure; + +public class SshInfrastructureFactoryTest { + + @Test + public void ignoresExtraHosts() { +SshInfrastructureFactory factory = +new SshInfrastructureFactory("user", "localhost", "localhost", "localhost"); +Infrastructure infra = factory.create(2); + +Assertions.assertThat(infra.getNodes()).hasSize(2); + } + +} This is an automated message from the Apache Git Service. To respond to the message, please log on 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 With regards, Apache Git Services
[GitHub] [geode-benchmarks] upthewaterspout closed pull request #13: Ignoring extra hosts in SshInfrastructure
[ pull request closed by upthewaterspout ] [ Full content available at: https://github.com/apache/geode-benchmarks/pull/13 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] pivotal-jbarrett commented on issue #2788: GEODE-5994: Add CPUContentionService
@dhemery can this PR be closed if it was moved to #2937? [ Full content available at: https://github.com/apache/geode/pull/2788 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] pivotal-jbarrett commented on issue #2563: GEODE-5787: support dunit VM bounce on Windows
@sboorlagadda are you waiting for something before merging these approved changes, besides resolving conflicts? [ Full content available at: https://github.com/apache/geode/pull/2563 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] pivotal-jbarrett commented on issue #2546: GEODE-5800: remove redundant casts for `DataSerializer.readObject()`
@galen-pivotal What you waiting for to merge this change? [ Full content available at: https://github.com/apache/geode/pull/2546 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] WireBaron closed pull request #14: GEODE-6145: Allow multiple results for a single probe analyzer
WireBaron closed pull request #14: GEODE-6145: Allow multiple results for a single probe analyzer URL: https://github.com/apache/geode-benchmarks/pull/14 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/harness/src/main/java/org/apache/geode/perftest/analysis/BenchmarkRunAnalyzer.java b/harness/src/main/java/org/apache/geode/perftest/analysis/BenchmarkRunAnalyzer.java index 764f622..57ae207 100644 --- a/harness/src/main/java/org/apache/geode/perftest/analysis/BenchmarkRunAnalyzer.java +++ b/harness/src/main/java/org/apache/geode/perftest/analysis/BenchmarkRunAnalyzer.java @@ -20,6 +20,7 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; +import java.util.Iterator; import java.util.List; import java.util.stream.Collectors; @@ -55,7 +56,7 @@ public void addProbe(ProbeResultParser probeResultParser) { probes.add(probeResultParser); } - public BenchmarkRunResult analyzeTestRun(File testResultDir, File baselineResultDir) + public BenchmarkRunResult analyzeTestRun(File baselineResultDir, File testResultDir) throws IOException { List benchmarkDirs = Arrays.asList(testResultDir.listFiles()); benchmarkDirs.sort(File::compareTo); @@ -72,22 +73,33 @@ public BenchmarkRunResult analyzeTestRun(File testResultDir, File baselineResult final BenchmarkRunResult.BenchmarkResult benchmarkResult = result.addBenchmark(testDir.getName()); for (ProbeResultParser probe : probes) { -double testResult = getTestResult(testYardstickDirs, probe); -double baselineResult = getTestResult(baselineYardstickDirs, probe); +List testResults = getTestResult(testYardstickDirs, probe); +List baselineResults = +getTestResult(baselineYardstickDirs, probe); -benchmarkResult.addProbeResult(probe.getResultDescription(), baselineResult, testResult); +Iterator testResultIter = testResults.iterator(); +Iterator baselineResultIter = baselineResults.iterator(); + +while (testResultIter.hasNext()) { + ProbeResultParser.ResultData testResult = testResultIter.next(); + ProbeResultParser.ResultData baselineResult = baselineResultIter.next(); + + benchmarkResult.addProbeResult(testResult.description, baselineResult.value, + testResult.value); +} } } return result; } - private double getTestResult(List resultDirs, ProbeResultParser probe) throws IOException { + private List getTestResult(List resultDirs, + ProbeResultParser probe) throws IOException { probe.reset(); for (File outputDirectory : resultDirs) { probe.parseResults(outputDirectory); } -return probe.getProbeResult(); +return probe.getProbeResults(); } private List getYardstickOutputForBenchmarkDir(File benchmarkDir) throws IOException { diff --git a/harness/src/main/java/org/apache/geode/perftest/analysis/ProbeResultParser.java b/harness/src/main/java/org/apache/geode/perftest/analysis/ProbeResultParser.java index 1e94329..dd1935e 100644 --- a/harness/src/main/java/org/apache/geode/perftest/analysis/ProbeResultParser.java +++ b/harness/src/main/java/org/apache/geode/perftest/analysis/ProbeResultParser.java @@ -16,8 +16,10 @@ import java.io.File; import java.io.IOException; +import java.util.List; public interface ProbeResultParser { + // Given a output directory for a benchmark, parse out the data for the desired probe. Note that // this method may be passed several csv files for a run and is expected to appropriately // aggregate the result of interest. @@ -26,9 +28,16 @@ // Reset the parser to a clean state where parseResults can be called again void reset(); - // Get a single float value summarizing the data for the probe. - double getProbeResult(); + // Get the {description, value} pairs for the probe + List getProbeResults(); + + class ResultData { +public String description; +public double value; - // Get a text description of what the probe result is depicting - String getResultDescription(); +public ResultData(String description, double value) { + this.description = description; + this.value = value; +} + } } diff --git a/harness/src/main/java/org/apache/geode/perftest/yardstick/analysis/YardstickHdrHistogramParser.java b/harness/src/main/java/org/apache/geode/perftest/yardstick/analysis/YardstickHdrHistogramParser.java index 8b2c6da..19d1749 100644 --- a/harness/src/main/java/org/apache/geode/perftest/yardstick/analysis/YardstickHdrHistogramParser.java +++ b/harness/src/main/java/org/apache/geode/perftest/yardstick/analysis/YardstickHdrHistogramParser.java @@ -17,6 +17,8 @@ import
[GitHub] [geode-benchmarks] WireBaron closed pull request #14: GEODE-6145: Allow multiple results for a single probe analyzer
[ pull request closed by WireBaron ] [ Full content available at: https://github.com/apache/geode-benchmarks/pull/14 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] kirklund closed pull request #2949: GEODE-6143: Change PowerMockito import to Mockito
[ pull request closed by kirklund ] [ Full content available at: https://github.com/apache/geode/pull/2949 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] samlanning commented on issue #2948: Fix ZipSlip bug found by LGTM.com
Thanks for merging @bschuchardt! Now that you've fixed lots of the alerts, have you considered enabling the [Automated Code Review](https://lgtm.com/projects/g/apache/geode/ci/)? [ Full content available at: https://github.com/apache/geode/pull/2948 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] pivotal-jbarrett commented on pull request #2915: GEODE-6117: Makes modules out of geode-core and geode-cq
It isn't, got left on accident. Thanks! [ Full content available at: https://github.com/apache/geode/pull/2915 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] pivotal-jbarrett commented on pull request #2915: GEODE-6117: Makes modules out of geode-core and geode-cq
Sure could. Was staying consistent with the other swizzle happening here. [ Full content available at: https://github.com/apache/geode/pull/2915 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] pivotal-jbarrett commented on pull request #2915: GEODE-6117: Makes modules out of geode-core and geode-cq
There is a class in the `geode-cq` that inherits from this inner class. Tt was able to before because it was in the same package but to support modules we can't split packages across jars anymore. In order for the `geode-cq` class to access this class it must now be public. Since it is "internal" it should not be considered public API. In fact it isn't exported in a way that any module other than `geode-cq` can access it in the `module-info.java`. [ Full content available at: https://github.com/apache/geode/pull/2915 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] karensmolermiller closed pull request #2942: GEODE-6127 Document changes to gfsh create jndi-binding command
[ pull request closed by karensmolermiller ] [ Full content available at: https://github.com/apache/geode/pull/2942 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] mcmellawatt commented on pull request #2915: GEODE-6117: Makes modules out of geode-core and geode-cq
This string and the one below it are used twice in this class. Maybe extract them into `static final` fields? [ Full content available at: https://github.com/apache/geode/pull/2915 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] mcmellawatt commented on pull request #2915: GEODE-6117: Makes modules out of geode-core and geode-cq
Not sure this comment is very helpful [ Full content available at: https://github.com/apache/geode/pull/2915 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] mcmellawatt commented on pull request #2915: GEODE-6117: Makes modules out of geode-core and geode-cq
Why does this need to be public now? I suppose it is part of an internal package, so does that mean we don't need to worry about this being a public API change? Mainly I'm just curious why this became public, I'm not seeing the corresponding usage that requires it to be public in this PR. [ Full content available at: https://github.com/apache/geode/pull/2915 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pdxcodemonkey closed pull request #413: GEODE-6139: Fix problems reported by rat in source release
[ pull request closed by pdxcodemonkey ] [ Full content available at: https://github.com/apache/geode-native/pull/413 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] bschuchardt closed pull request #2948: Fix ZipSlip bug found by LGTM.com
[ pull request closed by bschuchardt ] [ Full content available at: https://github.com/apache/geode/pull/2948 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org