Skip to content

Commit 58d80ec

Browse files
Don't log any percentage if logProgress is disabled
Co-authored-by: Ioannis Panagiotas <ioannis.panagiotas@neotechnology.com>
1 parent 23c866f commit 58d80ec

File tree

5 files changed

+44
-9
lines changed

5 files changed

+44
-9
lines changed

algo/src/test/java/org/neo4j/gds/similarity/nodesim/NodeSimilarityTest.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -816,15 +816,13 @@ void shouldNotLogMessagesWhenLoggingIsDisabled(int topK, int concurrency) {
816816
nodeSimilarity.compute();
817817

818818
assertThat(progressLog.getMessages(INFO))
819-
.as("When progress logging is disabled we only log `start`, `100%` and `finished`.")
819+
.as("When progress logging is disabled we only log `start` and `finished`.")
820820
.extracting(removingThreadId())
821821
.containsExactly(
822822
"NodeSimilarity :: Start",
823823
"NodeSimilarity :: prepare :: Start",
824-
"NodeSimilarity :: prepare 100%",
825824
"NodeSimilarity :: prepare :: Finished",
826825
"NodeSimilarity :: compare node pairs :: Start",
827-
"NodeSimilarity :: compare node pairs 100%",
828826
"NodeSimilarity :: compare node pairs :: Finished",
829827
"NodeSimilarity :: Finished"
830828
);

core/src/main/java/org/neo4j/gds/core/utils/progress/tasks/TaskProgressLogger.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,18 @@
2929
class TaskProgressLogger extends BatchingProgressLogger {
3030

3131
private final Task baseTask;
32-
private final LoggingLeafTaskVisitor loggingLeafTaskVisitor;
32+
private final TaskVisitor loggingLeafTaskVisitor;
3333

3434
TaskProgressLogger(Log log, Task baseTask, int concurrency) {
3535
super(log, baseTask, concurrency);
3636
this.baseTask = baseTask;
3737
this.loggingLeafTaskVisitor = new LoggingLeafTaskVisitor(this);
38+
39+
}
40+
TaskProgressLogger(Log log, Task baseTask, int concurrency, TaskVisitor leafTaskVisitor) {
41+
super(log, baseTask, concurrency);
42+
this.baseTask = baseTask;
43+
this.loggingLeafTaskVisitor = leafTaskVisitor;
3844
}
3945

4046
void logBeginSubTask(Task task, Task parentTask) {

core/src/main/java/org/neo4j/gds/core/utils/progress/tasks/TaskProgressTracker.java

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,12 +58,26 @@ public TaskProgressTracker(Task baseTask, Log log, int concurrency, TaskRegistry
5858
}
5959

6060
public TaskProgressTracker(
61-
Task baseTask, Log log, int concurrency, JobId jobId, TaskRegistryFactory taskRegistryFactory,
61+
Task baseTask,
62+
Log log,
63+
int concurrency,
64+
JobId jobId,
65+
TaskRegistryFactory taskRegistryFactory,
66+
UserLogRegistryFactory userLogRegistryFactory
67+
) {
68+
this(baseTask, jobId, taskRegistryFactory, new TaskProgressLogger(log, baseTask, concurrency), userLogRegistryFactory);
69+
}
70+
71+
protected TaskProgressTracker(
72+
Task baseTask,
73+
JobId jobId,
74+
TaskRegistryFactory taskRegistryFactory,
75+
TaskProgressLogger taskProgressLogger,
6276
UserLogRegistryFactory userLogRegistryFactory
6377
) {
6478
this.baseTask = baseTask;
6579
this.taskRegistry = taskRegistryFactory.newInstance(jobId);
66-
this.taskProgressLogger = new TaskProgressLogger(log, baseTask, concurrency);
80+
this.taskProgressLogger = taskProgressLogger;
6781
this.currentTask = Optional.empty();
6882
this.currentTotalSteps = UNKNOWN_STEPS;
6983
this.progressLeftOvers = 0;

core/src/main/java/org/neo4j/gds/core/utils/progress/tasks/TaskTreeProgressTracker.java

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,18 @@ public TaskTreeProgressTracker(
3434
TaskRegistryFactory taskRegistryFactory,
3535
UserLogRegistryFactory userLogRegistryFactory
3636
) {
37-
super(baseTask, log, concurrency, jobId, taskRegistryFactory, userLogRegistryFactory);
37+
super(
38+
baseTask,
39+
jobId,
40+
taskRegistryFactory,
41+
new TaskProgressLogger(
42+
log,
43+
baseTask,
44+
concurrency,
45+
new PassThroughTaskVisitor()
46+
),
47+
userLogRegistryFactory
48+
);
3849
}
3950

4051
@Override
@@ -51,4 +62,11 @@ public void logProgress(long value) {
5162
public void logProgress(long value, String messageTemplate) {
5263
// NOOP
5364
}
65+
66+
private static class PassThroughTaskVisitor implements TaskVisitor {
67+
@Override
68+
public void visitLeafTask(LeafTask leafTask) {
69+
// NOOP --> just pass through
70+
}
71+
}
5472
}

proc/test/src/test/java/org/neo4j/gds/test/ProgressTrackingTest.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,12 +85,11 @@ void shouldNotLogProgress() {
8585
factory.build(graph, testConfig, log, taskRegistryFactoryMock).compute();
8686

8787
assertThat(log.getMessages(TestLog.INFO))
88-
.as("When `logProgress` is set to `false` there should only be `start`, `100%` and `finished` log messages")
88+
.as("When `logProgress` is set to `false` there should only be `start` and `finished` log messages")
8989
.extracting(removingThreadId())
9090
.extracting(replaceTimings())
9191
.containsExactly(
9292
"TestAlgorithm :: Start",
93-
"TestAlgorithm 100%",
9493
"TestAlgorithm :: Finished"
9594
);
9695

0 commit comments

Comments
 (0)