Skip to content

Commit e24ec30

Browse files
rustyrussellrustyrussell
committed
autoclean: don't increment num_cleaned when record wasn't even a candidate.
For example, `autoclean-once failedforwards` would count every non-failed forwards as "uncleaned". This is both technically correct and completely useless. Changelog-Fixed: JSON-RPC: `autoclean-once` returns "uncleaned" number reflecting number of candidates which were too new to be cleaned, not all records we didn't delete. Fixes: #8632 Reported-by: @grubles and several other sharp-eyed users. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1 parent 07c57b6 commit e24ec30

File tree

4 files changed

+33
-31
lines changed

4 files changed

+33
-31
lines changed

‎contrib/msggen/msggen/schema.json‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1860,7 +1860,9 @@
18601860
" * `failedpays`: payment attempts which did not succeed (`failed` in listpays `status`).",
18611861
" * `succeededpays`: payment attempts which succeeded (`complete` in listpays `status`).",
18621862
" * `expiredinvoices`: invoices which were not paid (and cannot be) (`expired` in listinvoices `status`).",
1863-
" * `paidinvoices`: invoices which were paid (`paid` in listinvoices `status)."
1863+
" * `paidinvoices`: invoices which were paid (`paid` in listinvoices `status).",
1864+
"",
1865+
"NOTE: until v25.12, the `uncleaned` field contained all entries not removed (e.g. in `failedforwards` it counted all forwards, not just failed ones). This was an interface only an engineer could love, so it was fixed."
18641866
]
18651867
},
18661868
"age": {
@@ -1892,13 +1894,13 @@
18921894
"cleaned": {
18931895
"type": "u64",
18941896
"description": [
1895-
"Total number of deletions done this run."
1897+
"The number of successful forwards deleted."
18961898
]
18971899
},
18981900
"uncleaned": {
18991901
"type": "u64",
19001902
"description": [
1901-
"The total number of entries *not* deleted this run."
1903+
"The number of successful forwards *not* deleted (too new)."
19021904
]
19031905
}
19041906
}
@@ -1914,13 +1916,13 @@
19141916
"cleaned": {
19151917
"type": "u64",
19161918
"description": [
1917-
"Total number of deletions done this run."
1919+
"The number of failed forwards deleted."
19181920
]
19191921
},
19201922
"uncleaned": {
19211923
"type": "u64",
19221924
"description": [
1923-
"The total number of entries *not* deleted this run."
1925+
"The number of failed forwards *not* deleted (too new)."
19241926
]
19251927
}
19261928
}
@@ -1936,13 +1938,13 @@
19361938
"cleaned": {
19371939
"type": "u64",
19381940
"description": [
1939-
"Total number of deletions done this run."
1941+
"The number of successful payments deleted."
19401942
]
19411943
},
19421944
"uncleaned": {
19431945
"type": "u64",
19441946
"description": [
1945-
"The total number of entries *not* deleted this run."
1947+
"The number of successful forwards *not* deleted (too new)."
19461948
]
19471949
}
19481950
}
@@ -1958,13 +1960,13 @@
19581960
"cleaned": {
19591961
"type": "u64",
19601962
"description": [
1961-
"Total number of deletions done this run."
1963+
"The number of unsuccessful payments deleted."
19621964
]
19631965
},
19641966
"uncleaned": {
19651967
"type": "u64",
19661968
"description": [
1967-
"The total number of entries *not* deleted this run."
1969+
"The number of unsuccessful payments *not* deleted (too new)."
19681970
]
19691971
}
19701972
}
@@ -1980,13 +1982,13 @@
19801982
"cleaned": {
19811983
"type": "u64",
19821984
"description": [
1983-
"Total number of deletions done this run."
1985+
"The number of paid invoices deleted."
19841986
]
19851987
},
19861988
"uncleaned": {
19871989
"type": "u64",
19881990
"description": [
1989-
"The total number of entries *not* deleted this run."
1991+
"The number of paid invoices *not* deleted (too new)."
19901992
]
19911993
}
19921994
}
@@ -2002,13 +2004,13 @@
20022004
"cleaned": {
20032005
"type": "u64",
20042006
"description": [
2005-
"Total number of deletions done this run."
2007+
"The number of expired invoices deleted."
20062008
]
20072009
},
20082010
"uncleaned": {
20092011
"type": "u64",
20102012
"description": [
2011-
"The total number of entries *not* deleted this run."
2013+
"The number of expired invoices *not* deleted (too new)."
20122014
]
20132015
}
20142016
}

‎doc/schemas/autoclean-once.json‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@
3030
" * `failedpays`: payment attempts which did not succeed (`failed` in listpays `status`).",
3131
" * `succeededpays`: payment attempts which succeeded (`complete` in listpays `status`).",
3232
" * `expiredinvoices`: invoices which were not paid (and cannot be) (`expired` in listinvoices `status`).",
33-
" * `paidinvoices`: invoices which were paid (`paid` in listinvoices `status)."
33+
" * `paidinvoices`: invoices which were paid (`paid` in listinvoices `status).",
34+
"",
35+
"NOTE: until v25.12, the `uncleaned` field contained all entries not removed (e.g. in `failedforwards` it counted all forwards, not just failed ones). This was an interface only an engineer could love, so it was fixed."
3436
]
3537
},
3638
"age": {
@@ -62,13 +64,13 @@
6264
"cleaned": {
6365
"type": "u64",
6466
"description": [
65-
"Total number of deletions done this run."
67+
"The number of successful forwards deleted."
6668
]
6769
},
6870
"uncleaned": {
6971
"type": "u64",
7072
"description": [
71-
"The total number of entries *not* deleted this run."
73+
"The number of successful forwards *not* deleted (too new)."
7274
]
7375
}
7476
}
@@ -84,13 +86,13 @@
8486
"cleaned": {
8587
"type": "u64",
8688
"description": [
87-
"Total number of deletions done this run."
89+
"The number of failed forwards deleted."
8890
]
8991
},
9092
"uncleaned": {
9193
"type": "u64",
9294
"description": [
93-
"The total number of entries *not* deleted this run."
95+
"The number of failed forwards *not* deleted (too new)."
9496
]
9597
}
9698
}
@@ -106,13 +108,13 @@
106108
"cleaned": {
107109
"type": "u64",
108110
"description": [
109-
"Total number of deletions done this run."
111+
"The number of successful payments deleted."
110112
]
111113
},
112114
"uncleaned": {
113115
"type": "u64",
114116
"description": [
115-
"The total number of entries *not* deleted this run."
117+
"The number of successful forwards *not* deleted (too new)."
116118
]
117119
}
118120
}
@@ -128,13 +130,13 @@
128130
"cleaned": {
129131
"type": "u64",
130132
"description": [
131-
"Total number of deletions done this run."
133+
"The number of unsuccessful payments deleted."
132134
]
133135
},
134136
"uncleaned": {
135137
"type": "u64",
136138
"description": [
137-
"The total number of entries *not* deleted this run."
139+
"The number of unsuccessful payments *not* deleted (too new)."
138140
]
139141
}
140142
}
@@ -150,13 +152,13 @@
150152
"cleaned": {
151153
"type": "u64",
152154
"description": [
153-
"Total number of deletions done this run."
155+
"The number of paid invoices deleted."
154156
]
155157
},
156158
"uncleaned": {
157159
"type": "u64",
158160
"description": [
159-
"The total number of entries *not* deleted this run."
161+
"The number of paid invoices *not* deleted (too new)."
160162
]
161163
}
162164
}
@@ -172,13 +174,13 @@
172174
"cleaned": {
173175
"type": "u64",
174176
"description": [
175-
"Total number of deletions done this run."
177+
"The number of expired invoices deleted."
176178
]
177179
},
178180
"uncleaned": {
179181
"type": "u64",
180182
"description": [
181-
"The total number of entries *not* deleted this run."
183+
"The number of expired invoices *not* deleted (too new)."
182184
]
183185
}
184186
}

‎plugins/autoclean.c‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -511,13 +511,11 @@ static struct command_result *list_done(struct command *cmd,
511511

512512
variant = ops->get_variant(buf, t, subsystem, &timestamp);
513513
if (!variant) {
514-
subsystem->num_uncleaned++;
515514
continue;
516515
}
517516

518517
/* Continue if we don't care. */
519518
if (variant->age == 0) {
520-
subsystem->num_uncleaned++;
521519
continue;
522520
}
523521

‎tests/test_plugin.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3078,7 +3078,7 @@ def test_autoclean_once(node_factory):
30783078
# Make sure > 1 second old!
30793079
time.sleep(2)
30803080
assert (l1.rpc.autoclean_once('failedpays', 1)
3081-
== {'autoclean': {'failedpays': {'cleaned': 1, 'uncleaned': 1}}})
3081+
== {'autoclean': {'failedpays': {'cleaned': 1, 'uncleaned': 0}}})
30823082
assert l1.rpc.autoclean_status() == {'autoclean': {'failedpays': {'enabled': False,
30833083
'cleaned': 1},
30843084
'succeededpays': {'enabled': False,
@@ -3106,7 +3106,7 @@ def test_autoclean_once(node_factory):
31063106
'paidinvoices': {'enabled': False,
31073107
'cleaned': 0}}}
31083108
assert (l2.rpc.autoclean_once('failedforwards', 1)
3109-
== {'autoclean': {'failedforwards': {'cleaned': 1, 'uncleaned': 1}}})
3109+
== {'autoclean': {'failedforwards': {'cleaned': 1, 'uncleaned': 0}}})
31103110
assert l2.rpc.autoclean_status() == {'autoclean': {'failedpays': {'enabled': False,
31113111
'cleaned': 0},
31123112
'succeededpays': {'enabled': False,
@@ -3134,7 +3134,7 @@ def test_autoclean_once(node_factory):
31343134
'paidinvoices': {'enabled': False,
31353135
'cleaned': 0}}}
31363136
assert (l3.rpc.autoclean_once('expiredinvoices', 1)
3137-
== {'autoclean': {'expiredinvoices': {'cleaned': 1, 'uncleaned': 1}}})
3137+
== {'autoclean': {'expiredinvoices': {'cleaned': 1, 'uncleaned': 0}}})
31383138
assert l3.rpc.autoclean_status() == {'autoclean': {'failedpays': {'enabled': False,
31393139
'cleaned': 0},
31403140
'succeededpays': {'enabled': False,

0 commit comments

Comments
 (0)