From c3376ab13582b87f75adee3e1bd1fdeb8de6a3a6 Mon Sep 17 00:00:00 2001 From: Tom Boam Date: Thu, 2 Nov 2017 17:07:04 +0000 Subject: [PATCH 1/5] SweeperServiceImpl now logs when it starts sweeping make it clear if it is running full sweep or not --- .../atlasdb/sweep/SweeperServiceImpl.java | 33 ++++++++++++------- docs/source/release_notes/release-notes.rst | 3 ++ 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/sweep/SweeperServiceImpl.java b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/sweep/SweeperServiceImpl.java index dd65bcf05c6..6fd272452bc 100644 --- a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/sweep/SweeperServiceImpl.java +++ b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/sweep/SweeperServiceImpl.java @@ -17,13 +17,19 @@ import java.util.Optional; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import com.google.common.base.Preconditions; import com.palantir.atlasdb.encoding.PtBytes; import com.palantir.atlasdb.keyvalue.api.SweepResults; import com.palantir.atlasdb.keyvalue.api.TableReference; +import com.palantir.atlasdb.logging.LoggingArgs; import com.palantir.remoting3.servers.jersey.WebPreconditions; public final class SweeperServiceImpl implements SweeperService { + private static final Logger log = LoggerFactory.getLogger(SweeperServiceImpl.class); + private final SpecificTableSweeper specificTableSweeper; private final AdjustableSweepBatchConfigSource sweepBatchConfigSource; @@ -48,17 +54,22 @@ public SweepTableResponse sweepTable( SweepBatchConfig config = buildConfigWithOverrides(maxCellTsPairsToExamine, candidateBatchSize, deleteBatchSize); - SweepResults sweepResults = fullSweep.orElse(true) - ? runFullSweepWithoutSavingResults( - tableRef, - decodedStartRow, - config) - : runOneBatchWithoutSavingResults( - tableRef, - decodedStartRow, - config); - - return SweepTableResponse.from(sweepResults); + return SweepTableResponse.from(runSweep(fullSweep, tableRef, decodedStartRow, config)); + } + + private SweepResults runSweep(Optional fullSweep, TableReference tableRef, byte[] decodedStartRow, + SweepBatchConfig config) { + if (!fullSweep.isPresent()) { + log.warn("fullSweep parameter was not specified, defaulting to true"); + } + + if (fullSweep.orElse(true)) { + log.info("Running sweep of full table: {}", LoggingArgs.tableRef(tableRef)); + return runFullSweepWithoutSavingResults(tableRef, decodedStartRow, config); + } else { + log.info("Running sweep of a single batch on table: {}", LoggingArgs.tableRef(tableRef)); + return runOneBatchWithoutSavingResults(tableRef, decodedStartRow, config); + } } private SweepBatchConfig buildConfigWithOverrides( diff --git a/docs/source/release_notes/release-notes.rst b/docs/source/release_notes/release-notes.rst index 50084b07118..5362487ac53 100644 --- a/docs/source/release_notes/release-notes.rst +++ b/docs/source/release_notes/release-notes.rst @@ -48,6 +48,9 @@ develop * - Type - Change + * - |logs| + - ``SweeperServiceImpl`` now logs when it starts sweeping make it clear if it is running full sweep or not + * - - From b067262070a1fbc01636c9d5a0b134bb95089822 Mon Sep 17 00:00:00 2001 From: Tom Boam Date: Fri, 3 Nov 2017 21:04:53 +0000 Subject: [PATCH 2/5] Added sweep parameters to the log lines --- .../atlasdb/sweep/SweeperServiceImpl.java | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/sweep/SweeperServiceImpl.java b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/sweep/SweeperServiceImpl.java index 6fd272452bc..99db582e62f 100644 --- a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/sweep/SweeperServiceImpl.java +++ b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/sweep/SweeperServiceImpl.java @@ -25,6 +25,8 @@ import com.palantir.atlasdb.keyvalue.api.SweepResults; import com.palantir.atlasdb.keyvalue.api.TableReference; import com.palantir.atlasdb.logging.LoggingArgs; +import com.palantir.logsafe.SafeArg; +import com.palantir.logsafe.UnsafeArg; import com.palantir.remoting3.servers.jersey.WebPreconditions; public final class SweeperServiceImpl implements SweeperService { @@ -64,10 +66,24 @@ private SweepResults runSweep(Optional fullSweep, TableReference tableR } if (fullSweep.orElse(true)) { - log.info("Running sweep of full table: {}", LoggingArgs.tableRef(tableRef)); + log.info("Running sweep of full table {}, " + + "with maxCellTsPairsToExamine: {}, candidateBatchSize: {}, deleteBatchSize: {}, " + + "starting from row {}", + LoggingArgs.tableRef(tableRef), + SafeArg.of("maxCellTsPairsToExamine", config.maxCellTsPairsToExamine()), + SafeArg.of("candidateBatchSize", config.candidateBatchSize()), + SafeArg.of("deleteBatchSize", config.deleteBatchSize()), + UnsafeArg.of("decodedStartRow", decodedStartRow)); return runFullSweepWithoutSavingResults(tableRef, decodedStartRow, config); } else { - log.info("Running sweep of a single batch on table: {}", LoggingArgs.tableRef(tableRef)); + log.info("Running sweep of a single batch on table {}, " + + "with maxCellTsPairsToExamine: {}, candidateBatchSize: {}, deleteBatchSize: {}, " + + "starting from row {}", + LoggingArgs.tableRef(tableRef), + SafeArg.of("maxCellTsPairsToExamine", config.maxCellTsPairsToExamine()), + SafeArg.of("candidateBatchSize", config.candidateBatchSize()), + SafeArg.of("deleteBatchSize", config.deleteBatchSize()), + UnsafeArg.of("decodedStartRow", decodedStartRow)); return runOneBatchWithoutSavingResults(tableRef, decodedStartRow, config); } } From 19239bcbc6202de9442f81651299c77ae505861e Mon Sep 17 00:00:00 2001 From: Samuel Souza Date: Mon, 6 Nov 2017 10:40:23 +0000 Subject: [PATCH 3/5] Fix release notes --- docs/source/release_notes/release-notes.rst | 3 --- 1 file changed, 3 deletions(-) diff --git a/docs/source/release_notes/release-notes.rst b/docs/source/release_notes/release-notes.rst index f7eb6825c90..1d5c5812e30 100644 --- a/docs/source/release_notes/release-notes.rst +++ b/docs/source/release_notes/release-notes.rst @@ -47,9 +47,6 @@ develop * - Type - Change - - * - - - * - |improved| |logs| - ``SweeperServiceImpl`` now logs when it starts sweeping and makes it clear if it is running full sweep or not From c14c7b72af54cee38c3fbaed8aacf5f433b60640 Mon Sep 17 00:00:00 2001 From: Tom Boam Date: Tue, 7 Nov 2017 09:03:00 +0000 Subject: [PATCH 4/5] no longer default the service parameter in the interface, this way we can see when the parameter isn't provided and we are defaulting to true. Behaviour is unchanged but we can log a message when defaulting. --- .../main/java/com/palantir/atlasdb/sweep/SweeperService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/sweep/SweeperService.java b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/sweep/SweeperService.java index b9c9e69b27f..a89eddc232a 100644 --- a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/sweep/SweeperService.java +++ b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/sweep/SweeperService.java @@ -60,7 +60,7 @@ default SweepTableResponse sweepTableFrom(String tableName, String startRow) { SweepTableResponse sweepTable( @QueryParam("tablename") String tableName, @QueryParam("startRow") Optional startRow, - @Safe @QueryParam("fullSweep") @DefaultValue("true") Optional fullSweep, + @Safe @QueryParam("fullSweep") Optional fullSweep, @Safe @QueryParam("maxCellTsPairsToExamine") Optional maxCellTsPairsToExamine, @Safe @QueryParam("candidateBatchSize") Optional candidateBatchSize, @Safe @QueryParam("deleteBatchSize") Optional deleteBatchSize); From 3b89c73da78afa910649cf0c0ca8336f2d0ca351 Mon Sep 17 00:00:00 2001 From: Samuel Souza Date: Tue, 7 Nov 2017 17:24:33 +0000 Subject: [PATCH 5/5] Checkstyle --- .../src/main/java/com/palantir/atlasdb/sweep/SweeperService.java | 1 - 1 file changed, 1 deletion(-) diff --git a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/sweep/SweeperService.java b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/sweep/SweeperService.java index a89eddc232a..1a68449d9c4 100644 --- a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/sweep/SweeperService.java +++ b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/sweep/SweeperService.java @@ -18,7 +18,6 @@ import java.util.Optional; import javax.ws.rs.Consumes; -import javax.ws.rs.DefaultValue; import javax.ws.rs.POST; import javax.ws.rs.Path; import javax.ws.rs.Produces;