You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Ken Howe <kh...@pivotal.io> on 2017/05/15 19:50:22 UTC
Review Request 59287: GEODE-2420: Enable export logs size estimation
and user warning
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59287/
-----------------------------------------------------------
Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick Rhomberg.
Repository: geode
Description
-------
Adds 'export logs' option, --file-limit-size, to allow user to set
maximun size of the epxorted logs zip file.
When size checking is enabled (file-limit-size > 0) then the check
will also prevent filling up the disk on each member while consolidating
and filtering the logs.
Diffs
-----
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java 41c85d6421c8283163b70f2a560c8e4cbb02f2cc
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java 647fff2edd5cb7aae56ec994747354ad2adefd9a
geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java 06af662f21f2bcf9abec8fa25ff7ea6ddb38d1e4
geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogSizer.java 0a799f6c85dada2791da57585234fa2e47ef0b3d
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java a02c07f2c28156e097306f4b57174cddeda78845
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java 96ac76588662b1de5d5bf41c24ab115d90fc0a85
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java ec2bcfe8ea876172c6946c43c005659d23d055e0
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java 90a92f33247ecec8ee300ecb80a5d8ab27193c94
geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionCacheTest.java d8f2f2db937fc51ab5f917659e766f338b9ae847
Diff: https://reviews.apache.org/r/59287/diff/1/
Testing
-------
Precheckin is in progress - all green so far with only DistributedTest still running
Thanks,
Ken Howe
Re: Review Request 59287: GEODE-2420: Enable export logs size
estimation and user warning
Posted by Ken Howe <kh...@pivotal.io>.
> On May 15, 2017, 8:53 p.m., Kirk Lund wrote:
> > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java
> > Lines 160 (patched)
> > <https://reviews.apache.org/r/59287/diff/1/?file=1719248#file1719248line167>
> >
> > My merge to develop might cause you some weirdness. You might want to mock InternalCache instead but I'm not sure.
Correct. Changed to use InternalCache everywhere in ExportLogsCommandTest. Also change the access of ExportLogsCommand.getCache to package-private so it can spied it in the tests.
- Ken
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59287/#review175023
-----------------------------------------------------------
On May 22, 2017, 6:14 p.m., Ken Howe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59287/
> -----------------------------------------------------------
>
> (Updated May 22, 2017, 6:14 p.m.)
>
>
> Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Adds 'export logs' option, --file-limit-size, to allow user to set
> maximun size of the epxorted logs zip file.
>
> When size checking is enabled (file-limit-size > 0) then the check
> will also prevent filling up the disk on each member while consolidating
> and filtering the logs.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java 7170f209ffa169fb6efdc851d35b61a2031888b7
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java 20ec1f5702aea341ace5aa3b103c34cbdce1ae87
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java 663a08e15624ed3dbc032460133fe3c62fc5ac26
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfo.java c175e1ae3def869890692461bd129891350b383c
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java 8d20dc05c14bf558462893c4dd4cbbc474df4077
> geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java 68d055cbd61ca35ef7409ff3370214a005da3d9b
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogExporter.java a0be7fbd83918fccb5254b4a48ba7bf14a0fb344
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogSizer.java 0a799f6c85dada2791da57585234fa2e47ef0b3d
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java a02c07f2c28156e097306f4b57174cddeda78845
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java 96ac76588662b1de5d5bf41c24ab115d90fc0a85
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java ec2bcfe8ea876172c6946c43c005659d23d055e0
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java 90a92f33247ecec8ee300ecb80a5d8ab27193c94
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfoTest.java 0bfbefa90af7813a8cf20529d36c9cbe5111f9d9
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionCacheTest.java d8f2f2db937fc51ab5f917659e766f338b9ae847
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsTestSuite.java e70a750f48a8f7cc3b10b89be9b5934944addb0d
> geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogExporterTest.java a387af3b70f61256b5d9303de29e9402bbdd71e6
> geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogSizerTest.java c7b3ab934ce1cf97c0a70bc23b50f0f5ca08bb40
>
>
> Diff: https://reviews.apache.org/r/59287/diff/3/
>
>
> Testing
> -------
>
> Precheckin is in progress - all green so far with only DistributedTest still running
>
> 5/22 - Precheckin being re-run. Currently Green expcet for the known LocatorLauncher test failures. DistributedTests still running
>
>
> Thanks,
>
> Ken Howe
>
>
Re: Review Request 59287: GEODE-2420: Enable export logs size
estimation and user warning
Posted by Kirk Lund <ki...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59287/#review175023
-----------------------------------------------------------
Ship it!
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java
Lines 160 (patched)
<https://reviews.apache.org/r/59287/#comment248340>
My merge to develop might cause you some weirdness. You might want to mock InternalCache instead but I'm not sure.
- Kirk Lund
On May 15, 2017, 7:50 p.m., Ken Howe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59287/
> -----------------------------------------------------------
>
> (Updated May 15, 2017, 7:50 p.m.)
>
>
> Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Adds 'export logs' option, --file-limit-size, to allow user to set
> maximun size of the epxorted logs zip file.
>
> When size checking is enabled (file-limit-size > 0) then the check
> will also prevent filling up the disk on each member while consolidating
> and filtering the logs.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java 41c85d6421c8283163b70f2a560c8e4cbb02f2cc
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java 647fff2edd5cb7aae56ec994747354ad2adefd9a
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java 06af662f21f2bcf9abec8fa25ff7ea6ddb38d1e4
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogSizer.java 0a799f6c85dada2791da57585234fa2e47ef0b3d
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java a02c07f2c28156e097306f4b57174cddeda78845
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java 96ac76588662b1de5d5bf41c24ab115d90fc0a85
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java ec2bcfe8ea876172c6946c43c005659d23d055e0
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java 90a92f33247ecec8ee300ecb80a5d8ab27193c94
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionCacheTest.java d8f2f2db937fc51ab5f917659e766f338b9ae847
>
>
> Diff: https://reviews.apache.org/r/59287/diff/1/
>
>
> Testing
> -------
>
> Precheckin is in progress - all green so far with only DistributedTest still running
>
>
> Thanks,
>
> Ken Howe
>
>
Re: Review Request 59287: GEODE-2420: Enable export logs size
estimation and user warning
Posted by Ken Howe <kh...@pivotal.io>.
> On May 16, 2017, 5:19 p.m., Jared Stewart wrote:
> > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
> > Line 230 (original), 300 (patched)
> > <https://reviews.apache.org/r/59287/diff/2/?file=1720360#file1720360line305>
> >
> > It doesn't look like the return values of this method or the method on 319 are ever used in the product code. Does it make sense to have these return a boolean rather than void?
Reverted this method to a void type and refactored tests.
> On May 16, 2017, 5:19 p.m., Jared Stewart wrote:
> > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
> > Line 241 (original), 319 (patched)
> > <https://reviews.apache.org/r/59287/diff/2/?file=1720360#file1720360line324>
> >
> > The `diskSize` parameter in this method looks to be unused.
After refactoring to do the size check on on each member instead of back at the locator, the only item returned from SizeExportLogFunction is the estimated size for that member. Consequently, I've removed the ExportedLogsSizeInfo, and now just retiurn a single Long as the result (or error if the size check fails).
- Ken
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59287/#review175123
-----------------------------------------------------------
On May 22, 2017, 6:14 p.m., Ken Howe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59287/
> -----------------------------------------------------------
>
> (Updated May 22, 2017, 6:14 p.m.)
>
>
> Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Adds 'export logs' option, --file-limit-size, to allow user to set
> maximun size of the epxorted logs zip file.
>
> When size checking is enabled (file-limit-size > 0) then the check
> will also prevent filling up the disk on each member while consolidating
> and filtering the logs.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java 7170f209ffa169fb6efdc851d35b61a2031888b7
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java 20ec1f5702aea341ace5aa3b103c34cbdce1ae87
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java 663a08e15624ed3dbc032460133fe3c62fc5ac26
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfo.java c175e1ae3def869890692461bd129891350b383c
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java 8d20dc05c14bf558462893c4dd4cbbc474df4077
> geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java 68d055cbd61ca35ef7409ff3370214a005da3d9b
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogExporter.java a0be7fbd83918fccb5254b4a48ba7bf14a0fb344
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogSizer.java 0a799f6c85dada2791da57585234fa2e47ef0b3d
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java a02c07f2c28156e097306f4b57174cddeda78845
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java 96ac76588662b1de5d5bf41c24ab115d90fc0a85
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java ec2bcfe8ea876172c6946c43c005659d23d055e0
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java 90a92f33247ecec8ee300ecb80a5d8ab27193c94
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfoTest.java 0bfbefa90af7813a8cf20529d36c9cbe5111f9d9
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionCacheTest.java d8f2f2db937fc51ab5f917659e766f338b9ae847
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsTestSuite.java e70a750f48a8f7cc3b10b89be9b5934944addb0d
> geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogExporterTest.java a387af3b70f61256b5d9303de29e9402bbdd71e6
> geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogSizerTest.java c7b3ab934ce1cf97c0a70bc23b50f0f5ca08bb40
>
>
> Diff: https://reviews.apache.org/r/59287/diff/3/
>
>
> Testing
> -------
>
> Precheckin is in progress - all green so far with only DistributedTest still running
>
> 5/22 - Precheckin being re-run. Currently Green expcet for the known LocatorLauncher test failures. DistributedTests still running
>
>
> Thanks,
>
> Ken Howe
>
>
Re: Review Request 59287: GEODE-2420: Enable export logs size
estimation and user warning
Posted by Jared Stewart <js...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59287/#review175123
-----------------------------------------------------------
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
Line 230 (original), 300 (patched)
<https://reviews.apache.org/r/59287/#comment248490>
It doesn't look like the return values of this method or the method on 319 are ever used in the product code. Does it make sense to have these return a boolean rather than void?
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
Line 241 (original), 319 (patched)
<https://reviews.apache.org/r/59287/#comment248492>
The `diskSize` parameter in this method looks to be unused.
- Jared Stewart
On May 15, 2017, 10:09 p.m., Ken Howe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59287/
> -----------------------------------------------------------
>
> (Updated May 15, 2017, 10:09 p.m.)
>
>
> Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Adds 'export logs' option, --file-limit-size, to allow user to set
> maximun size of the epxorted logs zip file.
>
> When size checking is enabled (file-limit-size > 0) then the check
> will also prevent filling up the disk on each member while consolidating
> and filtering the logs.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java 41c85d6421c8283163b70f2a560c8e4cbb02f2cc
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java 20ec1f5702aea341ace5aa3b103c34cbdce1ae87
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java 8d20dc05c14bf558462893c4dd4cbbc474df4077
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogSizer.java 0a799f6c85dada2791da57585234fa2e47ef0b3d
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java a02c07f2c28156e097306f4b57174cddeda78845
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java 96ac76588662b1de5d5bf41c24ab115d90fc0a85
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java ec2bcfe8ea876172c6946c43c005659d23d055e0
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java 90a92f33247ecec8ee300ecb80a5d8ab27193c94
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionCacheTest.java d8f2f2db937fc51ab5f917659e766f338b9ae847
>
>
> Diff: https://reviews.apache.org/r/59287/diff/2/
>
>
> Testing
> -------
>
> Precheckin is in progress - all green so far with only DistributedTest still running
>
>
> Thanks,
>
> Ken Howe
>
>
Re: Review Request 59287: GEODE-2420: Enable export logs size
estimation and user warning
Posted by Ken Howe <kh...@pivotal.io>.
> On May 17, 2017, 5:38 p.m., Patrick Rhomberg wrote:
> > geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java
> > Line 28 (original), 28 (patched)
> > <https://reviews.apache.org/r/59287/diff/2/?file=1720359#file1720359line28>
> >
> > Are star-imports frowned upon? IntelliJ optimizes imports to...
> >
> > ```
> > import org.apache.geode.DataSerializer;
> > import org.apache.geode.GemFireConfigException;
> > import org.apache.geode.InternalGemFireError;
> > import org.apache.geode.cache.UnsupportedVersionException;
> > import org.apache.geode.distributed.DistributedMember;
> > import org.apache.geode.distributed.DurableClientAttributes;
> > import org.apache.geode.distributed.Role;
> > import org.apache.geode.distributed.internal.DistributionAdvisor.ProfileId;
> > import org.apache.geode.distributed.internal.DistributionConfig;
> > import org.apache.geode.distributed.internal.DistributionManager;
> > import org.apache.geode.distributed.internal.ServerLocation;
> > import org.apache.geode.internal.Assert;
> > import org.apache.geode.internal.DataSerializableFixedID;
> > import org.apache.geode.internal.InternalDataSerializer;
> > import org.apache.geode.internal.OSProcess;
> > import org.apache.geode.internal.Version;
> > import org.apache.geode.internal.cache.versions.VersionSource;
> > import org.apache.geode.internal.i18n.LocalizedStrings;
> > import org.apache.geode.internal.net.SocketCreator;
> >
> > import java.io.DataInput;
> > import java.io.DataOutput;
> > import java.io.EOFException;
> > import java.io.Externalizable;
> > import java.io.IOException;
> > import java.io.ObjectInput;
> > import java.io.ObjectOutput;
> > import java.net.InetAddress;
> > import java.net.UnknownHostException;
> > import java.util.Arrays;
> > import java.util.Collections;
> > import java.util.HashSet;
> > import java.util.List;
> > import java.util.Set;
> > ```
Did not make this change. The only changes in this file that I made were removing contributor's name from comments that I noticed while referencing the file during test development. Changes like this should be made on a separate ticket.
> On May 17, 2017, 5:38 p.m., Patrick Rhomberg wrote:
> > geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java
> > Line 1208 (original), 1208 (patched)
> > <https://reviews.apache.org/r/59287/diff/2/?file=1720359#file1720359line1208>
> >
> > InternalDistrubtedMemberWrapper -> InternalDistributedMemberWrapper
See reply above regarding changes to InternalDistributedMember
- Ken
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59287/#review175282
-----------------------------------------------------------
On May 22, 2017, 6:14 p.m., Ken Howe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59287/
> -----------------------------------------------------------
>
> (Updated May 22, 2017, 6:14 p.m.)
>
>
> Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Adds 'export logs' option, --file-limit-size, to allow user to set
> maximun size of the epxorted logs zip file.
>
> When size checking is enabled (file-limit-size > 0) then the check
> will also prevent filling up the disk on each member while consolidating
> and filtering the logs.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java 7170f209ffa169fb6efdc851d35b61a2031888b7
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java 20ec1f5702aea341ace5aa3b103c34cbdce1ae87
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java 663a08e15624ed3dbc032460133fe3c62fc5ac26
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfo.java c175e1ae3def869890692461bd129891350b383c
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java 8d20dc05c14bf558462893c4dd4cbbc474df4077
> geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java 68d055cbd61ca35ef7409ff3370214a005da3d9b
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogExporter.java a0be7fbd83918fccb5254b4a48ba7bf14a0fb344
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogSizer.java 0a799f6c85dada2791da57585234fa2e47ef0b3d
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java a02c07f2c28156e097306f4b57174cddeda78845
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java 96ac76588662b1de5d5bf41c24ab115d90fc0a85
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java ec2bcfe8ea876172c6946c43c005659d23d055e0
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java 90a92f33247ecec8ee300ecb80a5d8ab27193c94
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfoTest.java 0bfbefa90af7813a8cf20529d36c9cbe5111f9d9
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionCacheTest.java d8f2f2db937fc51ab5f917659e766f338b9ae847
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsTestSuite.java e70a750f48a8f7cc3b10b89be9b5934944addb0d
> geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogExporterTest.java a387af3b70f61256b5d9303de29e9402bbdd71e6
> geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogSizerTest.java c7b3ab934ce1cf97c0a70bc23b50f0f5ca08bb40
>
>
> Diff: https://reviews.apache.org/r/59287/diff/3/
>
>
> Testing
> -------
>
> Precheckin is in progress - all green so far with only DistributedTest still running
>
> 5/22 - Precheckin being re-run. Currently Green expcet for the known LocatorLauncher test failures. DistributedTests still running
>
>
> Thanks,
>
> Ken Howe
>
>
Re: Review Request 59287: GEODE-2420: Enable export logs size
estimation and user warning
Posted by Patrick Rhomberg <pr...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59287/#review175282
-----------------------------------------------------------
On a Mac, Alt-Shift-I does all the inspections for the current file. It's how I find all the typos that bother me so.
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java
Line 28 (original), 28 (patched)
<https://reviews.apache.org/r/59287/#comment248730>
Are star-imports frowned upon? IntelliJ optimizes imports to...
```
import org.apache.geode.DataSerializer;
import org.apache.geode.GemFireConfigException;
import org.apache.geode.InternalGemFireError;
import org.apache.geode.cache.UnsupportedVersionException;
import org.apache.geode.distributed.DistributedMember;
import org.apache.geode.distributed.DurableClientAttributes;
import org.apache.geode.distributed.Role;
import org.apache.geode.distributed.internal.DistributionAdvisor.ProfileId;
import org.apache.geode.distributed.internal.DistributionConfig;
import org.apache.geode.distributed.internal.DistributionManager;
import org.apache.geode.distributed.internal.ServerLocation;
import org.apache.geode.internal.Assert;
import org.apache.geode.internal.DataSerializableFixedID;
import org.apache.geode.internal.InternalDataSerializer;
import org.apache.geode.internal.OSProcess;
import org.apache.geode.internal.Version;
import org.apache.geode.internal.cache.versions.VersionSource;
import org.apache.geode.internal.i18n.LocalizedStrings;
import org.apache.geode.internal.net.SocketCreator;
import java.io.DataInput;
import java.io.DataOutput;
import java.io.EOFException;
import java.io.Externalizable;
import java.io.IOException;
import java.io.ObjectInput;
import java.io.ObjectOutput;
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
```
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java
Line 34 (original), 34 (patched)
<https://reviews.apache.org/r/59287/#comment248729>
This import can be removed.
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java
Line 1208 (original), 1208 (patched)
<https://reviews.apache.org/r/59287/#comment248727>
InternalDistrubtedMemberWrapper -> InternalDistributedMemberWrapper
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
Lines 158 (patched)
<https://reviews.apache.org/r/59287/#comment248737>
availble -> available
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java
Line 107 (original), 128 (patched)
<https://reviews.apache.org/r/59287/#comment248734>
mockFuntionExecutor -> mockFunctionExecutor
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java
Lines 139 (patched)
<https://reviews.apache.org/r/59287/#comment248735>
mockFuntionExecutor -> mockFunctionExecutor
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java
Lines 150 (patched)
<https://reviews.apache.org/r/59287/#comment248736>
mockFuntionExecutor -> mockFunctionExecutor
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java
Line 41 (original), 42 (patched)
<https://reviews.apache.org/r/59287/#comment248732>
This import can be removed.
geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionCacheTest.java
Lines 92-95 (original), 90-93 (patched)
<https://reviews.apache.org/r/59287/#comment248738>
initalFileSizes -> initialFileSizes
- Patrick Rhomberg
On May 15, 2017, 10:09 p.m., Ken Howe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59287/
> -----------------------------------------------------------
>
> (Updated May 15, 2017, 10:09 p.m.)
>
>
> Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Adds 'export logs' option, --file-limit-size, to allow user to set
> maximun size of the epxorted logs zip file.
>
> When size checking is enabled (file-limit-size > 0) then the check
> will also prevent filling up the disk on each member while consolidating
> and filtering the logs.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java 41c85d6421c8283163b70f2a560c8e4cbb02f2cc
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java 20ec1f5702aea341ace5aa3b103c34cbdce1ae87
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java 8d20dc05c14bf558462893c4dd4cbbc474df4077
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogSizer.java 0a799f6c85dada2791da57585234fa2e47ef0b3d
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java a02c07f2c28156e097306f4b57174cddeda78845
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java 96ac76588662b1de5d5bf41c24ab115d90fc0a85
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java ec2bcfe8ea876172c6946c43c005659d23d055e0
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java 90a92f33247ecec8ee300ecb80a5d8ab27193c94
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionCacheTest.java d8f2f2db937fc51ab5f917659e766f338b9ae847
>
>
> Diff: https://reviews.apache.org/r/59287/diff/2/
>
>
> Testing
> -------
>
> Precheckin is in progress - all green so far with only DistributedTest still running
>
>
> Thanks,
>
> Ken Howe
>
>
Re: Review Request 59287: GEODE-2420: Enable export logs size
estimation and user warning
Posted by Patrick Rhomberg <pr...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59287/#review175681
-----------------------------------------------------------
Ship it!
Ship It!
- Patrick Rhomberg
On May 22, 2017, 6:14 p.m., Ken Howe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59287/
> -----------------------------------------------------------
>
> (Updated May 22, 2017, 6:14 p.m.)
>
>
> Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Adds 'export logs' option, --file-limit-size, to allow user to set
> maximun size of the epxorted logs zip file.
>
> When size checking is enabled (file-limit-size > 0) then the check
> will also prevent filling up the disk on each member while consolidating
> and filtering the logs.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java 7170f209ffa169fb6efdc851d35b61a2031888b7
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java 20ec1f5702aea341ace5aa3b103c34cbdce1ae87
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java 663a08e15624ed3dbc032460133fe3c62fc5ac26
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfo.java c175e1ae3def869890692461bd129891350b383c
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java 8d20dc05c14bf558462893c4dd4cbbc474df4077
> geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java 68d055cbd61ca35ef7409ff3370214a005da3d9b
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogExporter.java a0be7fbd83918fccb5254b4a48ba7bf14a0fb344
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogSizer.java 0a799f6c85dada2791da57585234fa2e47ef0b3d
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java a02c07f2c28156e097306f4b57174cddeda78845
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java 96ac76588662b1de5d5bf41c24ab115d90fc0a85
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java ec2bcfe8ea876172c6946c43c005659d23d055e0
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java 90a92f33247ecec8ee300ecb80a5d8ab27193c94
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfoTest.java 0bfbefa90af7813a8cf20529d36c9cbe5111f9d9
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionCacheTest.java d8f2f2db937fc51ab5f917659e766f338b9ae847
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsTestSuite.java e70a750f48a8f7cc3b10b89be9b5934944addb0d
> geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogExporterTest.java a387af3b70f61256b5d9303de29e9402bbdd71e6
> geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogSizerTest.java c7b3ab934ce1cf97c0a70bc23b50f0f5ca08bb40
>
>
> Diff: https://reviews.apache.org/r/59287/diff/3/
>
>
> Testing
> -------
>
> Precheckin is in progress - all green so far with only DistributedTest still running
>
> 5/22 - Precheckin being re-run. Currently Green expcet for the known LocatorLauncher test failures. DistributedTests still running
>
>
> Thanks,
>
> Ken Howe
>
>
Re: Review Request 59287: GEODE-2420: Enable export logs size
estimation and user warning
Posted by Ken Howe <kh...@pivotal.io>.
> On May 22, 2017, 10:08 p.m., Jared Stewart wrote:
> > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
> > Line 300 (original), 268 (patched)
> > <https://reviews.apache.org/r/59287/diff/2-3/?file=1720360#file1720360line301>
> >
> > If I just saw the name of this method, I would expect it to return a boolean. Perhaps a name like `void throwIfFileSizeExceedsLimit` would make the intent more explicit.
Forgot to rename this when I changed the method type from boolean to void. Renamed similar to what Kirk suggested, checkFileSizeWithinLimit.
> On May 22, 2017, 10:08 p.m., Jared Stewart wrote:
> > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
> > Line 319 (original), 286 (patched)
> > <https://reviews.apache.org/r/59287/diff/2-3/?file=1720360#file1720360line324>
> >
> > As above, perhaps something like `void throwIfSizeExceedsDiskSpaceOfMember` would better indicate the intent of this method.
>
> Kirk Lund wrote:
> It's also common to name the method something like "checkFileSizeLimit"
Renamed to checkIfExportLogsOverflowsDisk. I updated the javadocs to explicitly call out the exception that's thrown.
- Ken
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59287/#review175719
-----------------------------------------------------------
On May 22, 2017, 6:14 p.m., Ken Howe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59287/
> -----------------------------------------------------------
>
> (Updated May 22, 2017, 6:14 p.m.)
>
>
> Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Adds 'export logs' option, --file-limit-size, to allow user to set
> maximun size of the epxorted logs zip file.
>
> When size checking is enabled (file-limit-size > 0) then the check
> will also prevent filling up the disk on each member while consolidating
> and filtering the logs.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java 7170f209ffa169fb6efdc851d35b61a2031888b7
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java 20ec1f5702aea341ace5aa3b103c34cbdce1ae87
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java 663a08e15624ed3dbc032460133fe3c62fc5ac26
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfo.java c175e1ae3def869890692461bd129891350b383c
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java 8d20dc05c14bf558462893c4dd4cbbc474df4077
> geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java 68d055cbd61ca35ef7409ff3370214a005da3d9b
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogExporter.java a0be7fbd83918fccb5254b4a48ba7bf14a0fb344
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogSizer.java 0a799f6c85dada2791da57585234fa2e47ef0b3d
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java a02c07f2c28156e097306f4b57174cddeda78845
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java 96ac76588662b1de5d5bf41c24ab115d90fc0a85
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java ec2bcfe8ea876172c6946c43c005659d23d055e0
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java 90a92f33247ecec8ee300ecb80a5d8ab27193c94
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfoTest.java 0bfbefa90af7813a8cf20529d36c9cbe5111f9d9
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionCacheTest.java d8f2f2db937fc51ab5f917659e766f338b9ae847
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsTestSuite.java e70a750f48a8f7cc3b10b89be9b5934944addb0d
> geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogExporterTest.java a387af3b70f61256b5d9303de29e9402bbdd71e6
> geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogSizerTest.java c7b3ab934ce1cf97c0a70bc23b50f0f5ca08bb40
>
>
> Diff: https://reviews.apache.org/r/59287/diff/3/
>
>
> Testing
> -------
>
> Precheckin is in progress - all green so far with only DistributedTest still running
>
> 5/22 - Precheckin being re-run. Currently Green expcet for the known LocatorLauncher test failures. DistributedTests still running
>
>
> Thanks,
>
> Ken Howe
>
>
Re: Review Request 59287: GEODE-2420: Enable export logs size
estimation and user warning
Posted by Kirk Lund <ki...@gmail.com>.
> On May 22, 2017, 10:08 p.m., Jared Stewart wrote:
> > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
> > Line 319 (original), 286 (patched)
> > <https://reviews.apache.org/r/59287/diff/2-3/?file=1720360#file1720360line324>
> >
> > As above, perhaps something like `void throwIfSizeExceedsDiskSpaceOfMember` would better indicate the intent of this method.
It's also common to name the method something like "checkFileSizeLimit"
- Kirk
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59287/#review175719
-----------------------------------------------------------
On May 22, 2017, 6:14 p.m., Ken Howe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59287/
> -----------------------------------------------------------
>
> (Updated May 22, 2017, 6:14 p.m.)
>
>
> Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Adds 'export logs' option, --file-limit-size, to allow user to set
> maximun size of the epxorted logs zip file.
>
> When size checking is enabled (file-limit-size > 0) then the check
> will also prevent filling up the disk on each member while consolidating
> and filtering the logs.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java 7170f209ffa169fb6efdc851d35b61a2031888b7
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java 20ec1f5702aea341ace5aa3b103c34cbdce1ae87
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java 663a08e15624ed3dbc032460133fe3c62fc5ac26
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfo.java c175e1ae3def869890692461bd129891350b383c
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java 8d20dc05c14bf558462893c4dd4cbbc474df4077
> geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java 68d055cbd61ca35ef7409ff3370214a005da3d9b
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogExporter.java a0be7fbd83918fccb5254b4a48ba7bf14a0fb344
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogSizer.java 0a799f6c85dada2791da57585234fa2e47ef0b3d
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java a02c07f2c28156e097306f4b57174cddeda78845
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java 96ac76588662b1de5d5bf41c24ab115d90fc0a85
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java ec2bcfe8ea876172c6946c43c005659d23d055e0
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java 90a92f33247ecec8ee300ecb80a5d8ab27193c94
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfoTest.java 0bfbefa90af7813a8cf20529d36c9cbe5111f9d9
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionCacheTest.java d8f2f2db937fc51ab5f917659e766f338b9ae847
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsTestSuite.java e70a750f48a8f7cc3b10b89be9b5934944addb0d
> geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogExporterTest.java a387af3b70f61256b5d9303de29e9402bbdd71e6
> geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogSizerTest.java c7b3ab934ce1cf97c0a70bc23b50f0f5ca08bb40
>
>
> Diff: https://reviews.apache.org/r/59287/diff/3/
>
>
> Testing
> -------
>
> Precheckin is in progress - all green so far with only DistributedTest still running
>
> 5/22 - Precheckin being re-run. Currently Green expcet for the known LocatorLauncher test failures. DistributedTests still running
>
>
> Thanks,
>
> Ken Howe
>
>
Re: Review Request 59287: GEODE-2420: Enable export logs size
estimation and user warning
Posted by Jared Stewart <js...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59287/#review175719
-----------------------------------------------------------
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
Line 300 (original), 268 (patched)
<https://reviews.apache.org/r/59287/#comment249069>
If I just saw the name of this method, I would expect it to return a boolean. Perhaps a name like `void throwIfFileSizeExceedsLimit` would make the intent more explicit.
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
Line 300 (original), 268 (patched)
<https://reviews.apache.org/r/59287/#comment249070>
If I just saw the name of this method, I would expect it to return a boolean. Perhaps a name like `void throwIfFileSizeExceedsLimit` would make the intent more explicit.
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
Line 319 (original), 286 (patched)
<https://reviews.apache.org/r/59287/#comment249071>
As above, perhaps something like `void throwIfSizeExceedsDiskSpaceOfMember` would better indicate the intent of this method.
- Jared Stewart
On May 22, 2017, 6:14 p.m., Ken Howe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59287/
> -----------------------------------------------------------
>
> (Updated May 22, 2017, 6:14 p.m.)
>
>
> Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Adds 'export logs' option, --file-limit-size, to allow user to set
> maximun size of the epxorted logs zip file.
>
> When size checking is enabled (file-limit-size > 0) then the check
> will also prevent filling up the disk on each member while consolidating
> and filtering the logs.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java 7170f209ffa169fb6efdc851d35b61a2031888b7
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java 20ec1f5702aea341ace5aa3b103c34cbdce1ae87
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java 663a08e15624ed3dbc032460133fe3c62fc5ac26
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfo.java c175e1ae3def869890692461bd129891350b383c
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java 8d20dc05c14bf558462893c4dd4cbbc474df4077
> geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java 68d055cbd61ca35ef7409ff3370214a005da3d9b
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogExporter.java a0be7fbd83918fccb5254b4a48ba7bf14a0fb344
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogSizer.java 0a799f6c85dada2791da57585234fa2e47ef0b3d
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java a02c07f2c28156e097306f4b57174cddeda78845
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java 96ac76588662b1de5d5bf41c24ab115d90fc0a85
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java ec2bcfe8ea876172c6946c43c005659d23d055e0
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java 90a92f33247ecec8ee300ecb80a5d8ab27193c94
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfoTest.java 0bfbefa90af7813a8cf20529d36c9cbe5111f9d9
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionCacheTest.java d8f2f2db937fc51ab5f917659e766f338b9ae847
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsTestSuite.java e70a750f48a8f7cc3b10b89be9b5934944addb0d
> geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogExporterTest.java a387af3b70f61256b5d9303de29e9402bbdd71e6
> geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogSizerTest.java c7b3ab934ce1cf97c0a70bc23b50f0f5ca08bb40
>
>
> Diff: https://reviews.apache.org/r/59287/diff/3/
>
>
> Testing
> -------
>
> Precheckin is in progress - all green so far with only DistributedTest still running
>
> 5/22 - Precheckin being re-run. Currently Green expcet for the known LocatorLauncher test failures. DistributedTests still running
>
>
> Thanks,
>
> Ken Howe
>
>
Re: Review Request 59287: GEODE-2420: Enable export logs size
estimation and user warning
Posted by Jinmei Liao <ji...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59287/#review175940
-----------------------------------------------------------
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
Lines 145 (patched)
<https://reviews.apache.org/r/59287/#comment249282>
is this method called anywhere else with a different first parameter?
If only called once, seems redundant that the method throws an exception and the caller directly catches that exception only to get the message. We can either directly build the message here or have the method return the message.
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
Lines 203 (patched)
<https://reviews.apache.org/r/59287/#comment249280>
We've already passed the size check before, and already generated the zip on the locator, I don't think we need this extra check here.
- Jinmei Liao
On May 23, 2017, 3:45 p.m., Ken Howe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59287/
> -----------------------------------------------------------
>
> (Updated May 23, 2017, 3:45 p.m.)
>
>
> Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Adds 'export logs' option, --file-limit-size, to allow user to set
> maximun size of the epxorted logs zip file.
>
> When size checking is enabled (file-limit-size > 0) then the check
> will also prevent filling up the disk on each member while consolidating
> and filtering the logs.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java 7170f209ffa169fb6efdc851d35b61a2031888b7
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java 20ec1f5702aea341ace5aa3b103c34cbdce1ae87
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java 663a08e15624ed3dbc032460133fe3c62fc5ac26
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfo.java c175e1ae3def869890692461bd129891350b383c
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java 8d20dc05c14bf558462893c4dd4cbbc474df4077
> geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java 68d055cbd61ca35ef7409ff3370214a005da3d9b
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogExporter.java a0be7fbd83918fccb5254b4a48ba7bf14a0fb344
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogSizer.java 0a799f6c85dada2791da57585234fa2e47ef0b3d
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java a02c07f2c28156e097306f4b57174cddeda78845
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java 96ac76588662b1de5d5bf41c24ab115d90fc0a85
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java ec2bcfe8ea876172c6946c43c005659d23d055e0
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java 90a92f33247ecec8ee300ecb80a5d8ab27193c94
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfoTest.java 0bfbefa90af7813a8cf20529d36c9cbe5111f9d9
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionCacheTest.java d8f2f2db937fc51ab5f917659e766f338b9ae847
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsTestSuite.java e70a750f48a8f7cc3b10b89be9b5934944addb0d
> geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogExporterTest.java a387af3b70f61256b5d9303de29e9402bbdd71e6
> geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogSizerTest.java c7b3ab934ce1cf97c0a70bc23b50f0f5ca08bb40
>
>
> Diff: https://reviews.apache.org/r/59287/diff/5/
>
>
> Testing
> -------
>
> Precheckin is in progress - all green so far with only DistributedTest still running
>
> 5/22 - Precheckin being re-run. Currently Green expcet for the known LocatorLauncher test failures. DistributedTests still running
>
> 5/23 - Re-running precheckin after syncing with current state of develop
>
>
> Thanks,
>
> Ken Howe
>
>
Re: Review Request 59287: GEODE-2420: Enable export logs size
estimation and user warning
Posted by Jared Stewart <js...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59287/#review175804
-----------------------------------------------------------
Ship it!
Ship It!
- Jared Stewart
On May 23, 2017, 3:45 p.m., Ken Howe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59287/
> -----------------------------------------------------------
>
> (Updated May 23, 2017, 3:45 p.m.)
>
>
> Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Adds 'export logs' option, --file-limit-size, to allow user to set
> maximun size of the epxorted logs zip file.
>
> When size checking is enabled (file-limit-size > 0) then the check
> will also prevent filling up the disk on each member while consolidating
> and filtering the logs.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java 7170f209ffa169fb6efdc851d35b61a2031888b7
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java 20ec1f5702aea341ace5aa3b103c34cbdce1ae87
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java 663a08e15624ed3dbc032460133fe3c62fc5ac26
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfo.java c175e1ae3def869890692461bd129891350b383c
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java 8d20dc05c14bf558462893c4dd4cbbc474df4077
> geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java 68d055cbd61ca35ef7409ff3370214a005da3d9b
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogExporter.java a0be7fbd83918fccb5254b4a48ba7bf14a0fb344
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogSizer.java 0a799f6c85dada2791da57585234fa2e47ef0b3d
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java a02c07f2c28156e097306f4b57174cddeda78845
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java 96ac76588662b1de5d5bf41c24ab115d90fc0a85
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java ec2bcfe8ea876172c6946c43c005659d23d055e0
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java 90a92f33247ecec8ee300ecb80a5d8ab27193c94
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfoTest.java 0bfbefa90af7813a8cf20529d36c9cbe5111f9d9
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionCacheTest.java d8f2f2db937fc51ab5f917659e766f338b9ae847
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsTestSuite.java e70a750f48a8f7cc3b10b89be9b5934944addb0d
> geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogExporterTest.java a387af3b70f61256b5d9303de29e9402bbdd71e6
> geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogSizerTest.java c7b3ab934ce1cf97c0a70bc23b50f0f5ca08bb40
>
>
> Diff: https://reviews.apache.org/r/59287/diff/5/
>
>
> Testing
> -------
>
> Precheckin is in progress - all green so far with only DistributedTest still running
>
> 5/22 - Precheckin being re-run. Currently Green expcet for the known LocatorLauncher test failures. DistributedTests still running
>
> 5/23 - Re-running precheckin after syncing with current state of develop
>
>
> Thanks,
>
> Ken Howe
>
>
Re: Review Request 59287: GEODE-2420: Enable export logs size
estimation and user warning
Posted by Kirk Lund <ki...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59287/#review175805
-----------------------------------------------------------
Ship it!
Ship It!
- Kirk Lund
On May 23, 2017, 3:45 p.m., Ken Howe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59287/
> -----------------------------------------------------------
>
> (Updated May 23, 2017, 3:45 p.m.)
>
>
> Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Adds 'export logs' option, --file-limit-size, to allow user to set
> maximun size of the epxorted logs zip file.
>
> When size checking is enabled (file-limit-size > 0) then the check
> will also prevent filling up the disk on each member while consolidating
> and filtering the logs.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java 7170f209ffa169fb6efdc851d35b61a2031888b7
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java 20ec1f5702aea341ace5aa3b103c34cbdce1ae87
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java 663a08e15624ed3dbc032460133fe3c62fc5ac26
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfo.java c175e1ae3def869890692461bd129891350b383c
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java 8d20dc05c14bf558462893c4dd4cbbc474df4077
> geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java 68d055cbd61ca35ef7409ff3370214a005da3d9b
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogExporter.java a0be7fbd83918fccb5254b4a48ba7bf14a0fb344
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogSizer.java 0a799f6c85dada2791da57585234fa2e47ef0b3d
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java a02c07f2c28156e097306f4b57174cddeda78845
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java 96ac76588662b1de5d5bf41c24ab115d90fc0a85
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java ec2bcfe8ea876172c6946c43c005659d23d055e0
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java 90a92f33247ecec8ee300ecb80a5d8ab27193c94
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfoTest.java 0bfbefa90af7813a8cf20529d36c9cbe5111f9d9
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionCacheTest.java d8f2f2db937fc51ab5f917659e766f338b9ae847
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsTestSuite.java e70a750f48a8f7cc3b10b89be9b5934944addb0d
> geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogExporterTest.java a387af3b70f61256b5d9303de29e9402bbdd71e6
> geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogSizerTest.java c7b3ab934ce1cf97c0a70bc23b50f0f5ca08bb40
>
>
> Diff: https://reviews.apache.org/r/59287/diff/5/
>
>
> Testing
> -------
>
> Precheckin is in progress - all green so far with only DistributedTest still running
>
> 5/22 - Precheckin being re-run. Currently Green expcet for the known LocatorLauncher test failures. DistributedTests still running
>
> 5/23 - Re-running precheckin after syncing with current state of develop
>
>
> Thanks,
>
> Ken Howe
>
>
Re: Review Request 59287: GEODE-2420: Enable export logs size
estimation and user warning
Posted by Ken Howe <kh...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59287/
-----------------------------------------------------------
(Updated May 23, 2017, 3:45 p.m.)
Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick Rhomberg.
Changes
-------
OK, ready to review now. Sorted out the repo sync and manually fixed the renames in the tests. The correct changes to review are between revisions 3 & 5. Ignore revision 4.
Repository: geode
Description
-------
Adds 'export logs' option, --file-limit-size, to allow user to set
maximun size of the epxorted logs zip file.
When size checking is enabled (file-limit-size > 0) then the check
will also prevent filling up the disk on each member while consolidating
and filtering the logs.
Diffs (updated)
-----
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java 7170f209ffa169fb6efdc851d35b61a2031888b7
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java 20ec1f5702aea341ace5aa3b103c34cbdce1ae87
geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java 663a08e15624ed3dbc032460133fe3c62fc5ac26
geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfo.java c175e1ae3def869890692461bd129891350b383c
geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java 8d20dc05c14bf558462893c4dd4cbbc474df4077
geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java 68d055cbd61ca35ef7409ff3370214a005da3d9b
geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogExporter.java a0be7fbd83918fccb5254b4a48ba7bf14a0fb344
geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogSizer.java 0a799f6c85dada2791da57585234fa2e47ef0b3d
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java a02c07f2c28156e097306f4b57174cddeda78845
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java 96ac76588662b1de5d5bf41c24ab115d90fc0a85
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java ec2bcfe8ea876172c6946c43c005659d23d055e0
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java 90a92f33247ecec8ee300ecb80a5d8ab27193c94
geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfoTest.java 0bfbefa90af7813a8cf20529d36c9cbe5111f9d9
geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionCacheTest.java d8f2f2db937fc51ab5f917659e766f338b9ae847
geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsTestSuite.java e70a750f48a8f7cc3b10b89be9b5934944addb0d
geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogExporterTest.java a387af3b70f61256b5d9303de29e9402bbdd71e6
geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogSizerTest.java c7b3ab934ce1cf97c0a70bc23b50f0f5ca08bb40
Diff: https://reviews.apache.org/r/59287/diff/5/
Changes: https://reviews.apache.org/r/59287/diff/4-5/
Testing
-------
Precheckin is in progress - all green so far with only DistributedTest still running
5/22 - Precheckin being re-run. Currently Green expcet for the known LocatorLauncher test failures. DistributedTests still running
5/23 - Re-running precheckin after syncing with current state of develop
Thanks,
Ken Howe
Re: Review Request 59287: GEODE-2420: Enable export logs size
estimation and user warning
Posted by Ken Howe <kh...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59287/#review175794
-----------------------------------------------------------
Hold off on reviewing this update! The IDE didn't handle the renames correctly, and the diff from my repo ended up with some extra changes.
- Ken Howe
On May 23, 2017, 2:52 p.m., Ken Howe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59287/
> -----------------------------------------------------------
>
> (Updated May 23, 2017, 2:52 p.m.)
>
>
> Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Adds 'export logs' option, --file-limit-size, to allow user to set
> maximun size of the epxorted logs zip file.
>
> When size checking is enabled (file-limit-size > 0) then the check
> will also prevent filling up the disk on each member while consolidating
> and filtering the logs.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/distributed/DistributedSystem.java 13a1721ae3967f6503f7a1042b4dddf246b83645
> geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java 7caad3f33ee55f72dc61c4adc2ab3de5429a1607
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java 7170f209ffa169fb6efdc851d35b61a2031888b7
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java 20ec1f5702aea341ace5aa3b103c34cbdce1ae87
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java 663a08e15624ed3dbc032460133fe3c62fc5ac26
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfo.java c175e1ae3def869890692461bd129891350b383c
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java 8d20dc05c14bf558462893c4dd4cbbc474df4077
> geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java 68d055cbd61ca35ef7409ff3370214a005da3d9b
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogExporter.java a0be7fbd83918fccb5254b4a48ba7bf14a0fb344
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogSizer.java 0a799f6c85dada2791da57585234fa2e47ef0b3d
> geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherTest.java d63da8755bed1041296d40ea9821251f01c4c59f
> geode-core/src/test/java/org/apache/geode/internal/cache/xmlcache/CacheXmlParserJUnitTest.java 4043888561d4206b1c0b405b3b77d5c9876ad99a
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java a02c07f2c28156e097306f4b57174cddeda78845
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java 96ac76588662b1de5d5bf41c24ab115d90fc0a85
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java ec2bcfe8ea876172c6946c43c005659d23d055e0
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java 90a92f33247ecec8ee300ecb80a5d8ab27193c94
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfoTest.java 0bfbefa90af7813a8cf20529d36c9cbe5111f9d9
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionCacheTest.java d8f2f2db937fc51ab5f917659e766f338b9ae847
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsTestSuite.java e70a750f48a8f7cc3b10b89be9b5934944addb0d
> geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogExporterTest.java a387af3b70f61256b5d9303de29e9402bbdd71e6
> geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogSizerTest.java c7b3ab934ce1cf97c0a70bc23b50f0f5ca08bb40
>
>
> Diff: https://reviews.apache.org/r/59287/diff/4/
>
>
> Testing
> -------
>
> Precheckin is in progress - all green so far with only DistributedTest still running
>
> 5/22 - Precheckin being re-run. Currently Green expcet for the known LocatorLauncher test failures. DistributedTests still running
>
> 5/23 - Re-running precheckin after syncing with current state of develop
>
>
> Thanks,
>
> Ken Howe
>
>
Re: Review Request 59287: GEODE-2420: Enable export logs size
estimation and user warning
Posted by Ken Howe <kh...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59287/
-----------------------------------------------------------
(Updated May 23, 2017, 2:52 p.m.)
Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick Rhomberg.
Changes
-------
renamed methods and updated javadocs
Repository: geode
Description
-------
Adds 'export logs' option, --file-limit-size, to allow user to set
maximun size of the epxorted logs zip file.
When size checking is enabled (file-limit-size > 0) then the check
will also prevent filling up the disk on each member while consolidating
and filtering the logs.
Diffs (updated)
-----
geode-core/src/main/java/org/apache/geode/distributed/DistributedSystem.java 13a1721ae3967f6503f7a1042b4dddf246b83645
geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java 7caad3f33ee55f72dc61c4adc2ab3de5429a1607
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java 7170f209ffa169fb6efdc851d35b61a2031888b7
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java 20ec1f5702aea341ace5aa3b103c34cbdce1ae87
geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java 663a08e15624ed3dbc032460133fe3c62fc5ac26
geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfo.java c175e1ae3def869890692461bd129891350b383c
geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java 8d20dc05c14bf558462893c4dd4cbbc474df4077
geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java 68d055cbd61ca35ef7409ff3370214a005da3d9b
geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogExporter.java a0be7fbd83918fccb5254b4a48ba7bf14a0fb344
geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogSizer.java 0a799f6c85dada2791da57585234fa2e47ef0b3d
geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherTest.java d63da8755bed1041296d40ea9821251f01c4c59f
geode-core/src/test/java/org/apache/geode/internal/cache/xmlcache/CacheXmlParserJUnitTest.java 4043888561d4206b1c0b405b3b77d5c9876ad99a
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java a02c07f2c28156e097306f4b57174cddeda78845
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java 96ac76588662b1de5d5bf41c24ab115d90fc0a85
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java ec2bcfe8ea876172c6946c43c005659d23d055e0
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java 90a92f33247ecec8ee300ecb80a5d8ab27193c94
geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfoTest.java 0bfbefa90af7813a8cf20529d36c9cbe5111f9d9
geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionCacheTest.java d8f2f2db937fc51ab5f917659e766f338b9ae847
geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsTestSuite.java e70a750f48a8f7cc3b10b89be9b5934944addb0d
geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogExporterTest.java a387af3b70f61256b5d9303de29e9402bbdd71e6
geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogSizerTest.java c7b3ab934ce1cf97c0a70bc23b50f0f5ca08bb40
Diff: https://reviews.apache.org/r/59287/diff/4/
Changes: https://reviews.apache.org/r/59287/diff/3-4/
Testing (updated)
-------
Precheckin is in progress - all green so far with only DistributedTest still running
5/22 - Precheckin being re-run. Currently Green expcet for the known LocatorLauncher test failures. DistributedTests still running
5/23 - Re-running precheckin after syncing with current state of develop
Thanks,
Ken Howe
Re: Review Request 59287: GEODE-2420: Enable export logs size
estimation and user warning
Posted by Ken Howe <kh...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59287/
-----------------------------------------------------------
(Updated May 22, 2017, 6:14 p.m.)
Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick Rhomberg.
Changes
-------
Updats from reviews.
Repository: geode
Description
-------
Adds 'export logs' option, --file-limit-size, to allow user to set
maximun size of the epxorted logs zip file.
When size checking is enabled (file-limit-size > 0) then the check
will also prevent filling up the disk on each member while consolidating
and filtering the logs.
Diffs (updated)
-----
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java 7170f209ffa169fb6efdc851d35b61a2031888b7
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java 20ec1f5702aea341ace5aa3b103c34cbdce1ae87
geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java 663a08e15624ed3dbc032460133fe3c62fc5ac26
geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfo.java c175e1ae3def869890692461bd129891350b383c
geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java 8d20dc05c14bf558462893c4dd4cbbc474df4077
geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java 68d055cbd61ca35ef7409ff3370214a005da3d9b
geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogExporter.java a0be7fbd83918fccb5254b4a48ba7bf14a0fb344
geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogSizer.java 0a799f6c85dada2791da57585234fa2e47ef0b3d
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java a02c07f2c28156e097306f4b57174cddeda78845
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java 96ac76588662b1de5d5bf41c24ab115d90fc0a85
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java ec2bcfe8ea876172c6946c43c005659d23d055e0
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java 90a92f33247ecec8ee300ecb80a5d8ab27193c94
geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfoTest.java 0bfbefa90af7813a8cf20529d36c9cbe5111f9d9
geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionCacheTest.java d8f2f2db937fc51ab5f917659e766f338b9ae847
geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsTestSuite.java e70a750f48a8f7cc3b10b89be9b5934944addb0d
geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogExporterTest.java a387af3b70f61256b5d9303de29e9402bbdd71e6
geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogSizerTest.java c7b3ab934ce1cf97c0a70bc23b50f0f5ca08bb40
Diff: https://reviews.apache.org/r/59287/diff/3/
Changes: https://reviews.apache.org/r/59287/diff/2-3/
Testing (updated)
-------
Precheckin is in progress - all green so far with only DistributedTest still running
5/22 - Precheckin being re-run. Currently Green expcet for the known LocatorLauncher test failures. DistributedTests still running
Thanks,
Ken Howe
Re: Review Request 59287: GEODE-2420: Enable export logs size
estimation and user warning
Posted by Ken Howe <kh...@pivotal.io>.
> On May 16, 2017, 5:59 p.m., Jinmei Liao wrote:
> > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
> > Lines 149 (patched)
> > <https://reviews.apache.org/r/59287/diff/2/?file=1720360#file1720360line154>
> >
> > looks like the locator decides if each server's exported log size exeedes the limit of each server, should we push this check onto the server itself?
Changed to do the size check on each member.
> On May 16, 2017, 5:59 p.m., Jinmei Liao wrote:
> > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
> > Lines 167 (patched)
> > <https://reviews.apache.org/r/59287/diff/2/?file=1720360#file1720360line172>
> >
> > This seems like a hacky way to do testing. I usally hate to see test code mixed up in the production code.
Removed the test hook, and moved a Unit test to and Integration test of SizeExportLogsFunction.
> On May 16, 2017, 5:59 p.m., Jinmei Liao wrote:
> > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
> > Line 241 (original), 319 (patched)
> > <https://reviews.apache.org/r/59287/diff/2/?file=1720360#file1720360line324>
> >
> > the returned boolean doesn't seem to be used. It looks like this function throws an exception if exeeding limit.
See note from jstewart's review
- Ken
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59287/#review175127
-----------------------------------------------------------
On May 22, 2017, 6:14 p.m., Ken Howe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59287/
> -----------------------------------------------------------
>
> (Updated May 22, 2017, 6:14 p.m.)
>
>
> Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Adds 'export logs' option, --file-limit-size, to allow user to set
> maximun size of the epxorted logs zip file.
>
> When size checking is enabled (file-limit-size > 0) then the check
> will also prevent filling up the disk on each member while consolidating
> and filtering the logs.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java 7170f209ffa169fb6efdc851d35b61a2031888b7
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java 20ec1f5702aea341ace5aa3b103c34cbdce1ae87
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java 663a08e15624ed3dbc032460133fe3c62fc5ac26
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfo.java c175e1ae3def869890692461bd129891350b383c
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java 8d20dc05c14bf558462893c4dd4cbbc474df4077
> geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java 68d055cbd61ca35ef7409ff3370214a005da3d9b
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogExporter.java a0be7fbd83918fccb5254b4a48ba7bf14a0fb344
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogSizer.java 0a799f6c85dada2791da57585234fa2e47ef0b3d
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java a02c07f2c28156e097306f4b57174cddeda78845
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java 96ac76588662b1de5d5bf41c24ab115d90fc0a85
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java ec2bcfe8ea876172c6946c43c005659d23d055e0
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java 90a92f33247ecec8ee300ecb80a5d8ab27193c94
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfoTest.java 0bfbefa90af7813a8cf20529d36c9cbe5111f9d9
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionCacheTest.java d8f2f2db937fc51ab5f917659e766f338b9ae847
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsTestSuite.java e70a750f48a8f7cc3b10b89be9b5934944addb0d
> geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogExporterTest.java a387af3b70f61256b5d9303de29e9402bbdd71e6
> geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogSizerTest.java c7b3ab934ce1cf97c0a70bc23b50f0f5ca08bb40
>
>
> Diff: https://reviews.apache.org/r/59287/diff/3/
>
>
> Testing
> -------
>
> Precheckin is in progress - all green so far with only DistributedTest still running
>
> 5/22 - Precheckin being re-run. Currently Green expcet for the known LocatorLauncher test failures. DistributedTests still running
>
>
> Thanks,
>
> Ken Howe
>
>
Re: Review Request 59287: GEODE-2420: Enable export logs size
estimation and user warning
Posted by Jinmei Liao <ji...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59287/#review175127
-----------------------------------------------------------
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
Lines 149 (patched)
<https://reviews.apache.org/r/59287/#comment248501>
looks like the locator decides if each server's exported log size exeedes the limit of each server, should we push this check onto the server itself?
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
Lines 167 (patched)
<https://reviews.apache.org/r/59287/#comment248495>
This seems like a hacky way to do testing. I usally hate to see test code mixed up in the production code.
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
Line 241 (original), 319 (patched)
<https://reviews.apache.org/r/59287/#comment248499>
the returned boolean doesn't seem to be used. It looks like this function throws an exception if exeeding limit.
geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogSizer.java
Line 48 (original), 50 (patched)
<https://reviews.apache.org/r/59287/#comment248497>
This seems to have some duplicate code with LogExporter. To make code concise and easy to maintain, I would combine these two classes together and call size() or export() at different times.
- Jinmei Liao
On May 15, 2017, 10:09 p.m., Ken Howe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59287/
> -----------------------------------------------------------
>
> (Updated May 15, 2017, 10:09 p.m.)
>
>
> Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Adds 'export logs' option, --file-limit-size, to allow user to set
> maximun size of the epxorted logs zip file.
>
> When size checking is enabled (file-limit-size > 0) then the check
> will also prevent filling up the disk on each member while consolidating
> and filtering the logs.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java 41c85d6421c8283163b70f2a560c8e4cbb02f2cc
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java 20ec1f5702aea341ace5aa3b103c34cbdce1ae87
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java 8d20dc05c14bf558462893c4dd4cbbc474df4077
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogSizer.java 0a799f6c85dada2791da57585234fa2e47ef0b3d
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java a02c07f2c28156e097306f4b57174cddeda78845
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java 96ac76588662b1de5d5bf41c24ab115d90fc0a85
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java ec2bcfe8ea876172c6946c43c005659d23d055e0
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java 90a92f33247ecec8ee300ecb80a5d8ab27193c94
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionCacheTest.java d8f2f2db937fc51ab5f917659e766f338b9ae847
>
>
> Diff: https://reviews.apache.org/r/59287/diff/2/
>
>
> Testing
> -------
>
> Precheckin is in progress - all green so far with only DistributedTest still running
>
>
> Thanks,
>
> Ken Howe
>
>
Re: Review Request 59287: GEODE-2420: Enable export logs size
estimation and user warning
Posted by Ken Howe <kh...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59287/
-----------------------------------------------------------
(Updated May 15, 2017, 10:09 p.m.)
Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick Rhomberg.
Repository: geode
Description
-------
Adds 'export logs' option, --file-limit-size, to allow user to set
maximun size of the epxorted logs zip file.
When size checking is enabled (file-limit-size > 0) then the check
will also prevent filling up the disk on each member while consolidating
and filtering the logs.
Diffs (updated)
-----
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java 41c85d6421c8283163b70f2a560c8e4cbb02f2cc
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java 20ec1f5702aea341ace5aa3b103c34cbdce1ae87
geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java 8d20dc05c14bf558462893c4dd4cbbc474df4077
geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogSizer.java 0a799f6c85dada2791da57585234fa2e47ef0b3d
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java a02c07f2c28156e097306f4b57174cddeda78845
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java 96ac76588662b1de5d5bf41c24ab115d90fc0a85
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java ec2bcfe8ea876172c6946c43c005659d23d055e0
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java 90a92f33247ecec8ee300ecb80a5d8ab27193c94
geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionCacheTest.java d8f2f2db937fc51ab5f917659e766f338b9ae847
Diff: https://reviews.apache.org/r/59287/diff/2/
Changes: https://reviews.apache.org/r/59287/diff/1-2/
Testing
-------
Precheckin is in progress - all green so far with only DistributedTest still running
Thanks,
Ken Howe