Mailing-List: contact dev-help@geode.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@geode.apache.org
Content-Type: multipart/alternative;
 boundary="===============8145984775312772246=="
MIME-Version: 1.0
Subject: Re: Review Request 59287: GEODE-2420: Enable export logs size
 estimation and user warning
From: Kirk Lund <kirk.lund@gmail.com>
To: Jinmei Liao <jiliao@pivotal.io>, Patrick Rhomberg <prhomberg@pivotal.io>,
 Kirk Lund <kirk.lund@gmail.com>, Jared Stewart <jstewart@pivotal.io>
Cc: geode <dev@geode.apache.org>, Ken Howe <khowe@pivotal.io>
Date: Mon, 22 May 2017 23:46:00 -0000
Message-ID: <20170522234600.58535.52274@reviews-vm2.apache.org>
Auto-Submitted: auto-generated
Sender: Kirk Lund <noreply@reviews.apache.org>
References: <20170522220818.58535.32686@reviews-vm2.apache.org>
In-Reply-To: <20170522220818.58535.32686@reviews-vm2.apache.org>
Reply-To: Kirk Lund <kirk.lund@gmail.com>
archived-at: Mon, 22 May 2017 23:46:06 -0000

--===============8145984775312772246==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


> 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
> 
>


--===============8145984775312772246==--