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