Repository navigation
fix(config): correct BlockTime cron evaluation and stop handling #7082
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: release_v4.8.3
Are you sure you want to change the base?
Changes from all commits
f25881e
1eef8cc
9088d97
344e86f
8b2e500
4e677ce
ee69633
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,8 +20,11 @@ | |
|
|
||
| import java.io.Serializable; | ||
| import java.text.ParseException; | ||
| import java.time.DayOfWeek; | ||
| import java.time.LocalDate; | ||
| import java.util.Calendar; | ||
| import java.util.Date; | ||
| import java.util.GregorianCalendar; | ||
| import java.util.HashMap; | ||
| import java.util.Locale; | ||
| import java.util.Map; | ||
|
|
@@ -205,7 +208,7 @@ | |
| */ | ||
| public final class CronExpression implements Serializable { | ||
|
|
||
| public static final int MAX_YEAR = Calendar.getInstance().get(Calendar.YEAR) + 100; | ||
| public static final int MAX_YEAR = new GregorianCalendar().get(Calendar.YEAR) + 100; | ||
| private static final int SECOND = 0; | ||
| private static final int MINUTE = 1; | ||
| private static final int HOUR = 2; | ||
|
|
@@ -573,6 +576,9 @@ private int storeExpressionVals(int pos, String s, int type) | |
| addToSet(ALL_SPEC_INT, -1, incr, type); | ||
| return i; | ||
| } else if (c == 'L') { | ||
| if (type < DAY_OF_MONTH) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [QUESTION] Will the strict-parsing follow-up also cover |
||
| throw new ParseException("'L' not expected in seconds, minutes or hours fields.", i); | ||
| } | ||
| i++; | ||
| if (type == DAY_OF_WEEK) { | ||
| addToSet(7, 7, 0, type); | ||
|
|
@@ -631,9 +637,9 @@ private int storeExpressionVals(int pos, String s, int type) | |
|
|
||
| private void checkIncrementRange(int incr, int type, int idxPos) throws ParseException { | ||
| if (incr > 59 && (type == SECOND || type == MINUTE)) { | ||
| throw new ParseException("Increment > 60 : " + incr, idxPos); | ||
| throw new ParseException("Increment >= 60 : " + incr, idxPos); | ||
| } else if (incr > 23 && (type == HOUR)) { | ||
| throw new ParseException("Increment > 24 : " + incr, idxPos); | ||
| throw new ParseException("Increment >= 24 : " + incr, idxPos); | ||
| } else if (incr > 31 && (type == DAY_OF_MONTH)) { | ||
| throw new ParseException("Increment > 31 : " + incr, idxPos); | ||
| } else if (incr > 7 && (type == DAY_OF_WEEK)) { | ||
|
|
@@ -1092,45 +1098,8 @@ public Date getTimeAfter(Date afterTime) { | |
| if (dayOfMSpec && !dayOfWSpec) { // get day by day of month rule | ||
| final Optional<Integer> smallestDay = findSmallestDay(day, mon, cl.get(Calendar.YEAR), | ||
| daysOfMonth); | ||
| Optional<Integer> smallestDayForWeekday = findSmallestDay(day, mon, cl.get(Calendar.YEAR), | ||
| nearestWeekdays); | ||
| t = day; | ||
| day = -1; | ||
| if (smallestDayForWeekday.isPresent()) { | ||
| day = smallestDayForWeekday.get(); | ||
|
|
||
| java.util.Calendar tcal = java.util.Calendar.getInstance(getTimeZone()); | ||
| tcal.set(Calendar.SECOND, 0); | ||
| tcal.set(Calendar.MINUTE, 0); | ||
| tcal.set(Calendar.HOUR_OF_DAY, 0); | ||
| tcal.set(Calendar.DAY_OF_MONTH, day); | ||
| tcal.set(Calendar.MONTH, mon - 1); | ||
| tcal.set(Calendar.YEAR, cl.get(Calendar.YEAR)); | ||
|
|
||
| int ldom = getLastDayOfMonth(mon, cl.get(Calendar.YEAR)); | ||
| int dow = tcal.get(Calendar.DAY_OF_WEEK); | ||
|
|
||
| if (dow == Calendar.SATURDAY && day == 1) { | ||
| day += 2; | ||
| } else if (dow == Calendar.SATURDAY) { | ||
| day -= 1; | ||
| } else if (dow == Calendar.SUNDAY && day == ldom) { | ||
| day -= 2; | ||
| } else if (dow == Calendar.SUNDAY) { | ||
| day += 1; | ||
| } | ||
|
|
||
|
|
||
| tcal.set(Calendar.SECOND, sec); | ||
| tcal.set(Calendar.MINUTE, min); | ||
| tcal.set(Calendar.HOUR_OF_DAY, hr); | ||
| tcal.set(Calendar.DAY_OF_MONTH, day); | ||
| tcal.set(Calendar.MONTH, mon - 1); | ||
| Date nTime = tcal.getTime(); | ||
| if (nTime.before(afterTime)) { | ||
| day = -1; | ||
| } | ||
| } | ||
| day = findNearestWeekday(t, mon, cl.get(Calendar.YEAR)); | ||
| if (smallestDay.isPresent()) { | ||
| if (day == -1 || smallestDay.get() < day) { | ||
| day = smallestDay.get(); | ||
|
|
@@ -1395,17 +1364,41 @@ private Optional<Integer> findSmallestDay(int day, int mon, int year, TreeSet<In | |
|
|
||
| final int lastDay = getLastDayOfMonth(mon, year); | ||
| // For "L", "L-1", etc. | ||
| int smallestDay = Optional.ofNullable(set.ceiling(LAST_DAY_OFFSET_END - (lastDay - day))) | ||
| .map(d -> d - LAST_DAY_OFFSET_START + 1) | ||
| final int smallestDay = Optional.ofNullable(set.ceiling(LAST_DAY_OFFSET_END - (lastDay - day))) | ||
| .map(d -> lastDay - (LAST_DAY_OFFSET_END - d)) | ||
| .orElse(Integer.MAX_VALUE); | ||
|
|
||
| // For "1", "2", etc. | ||
| SortedSet<Integer> st = set.subSet(day, LAST_DAY_OFFSET_START); | ||
| // make sure we don't over-run a short month, such as february | ||
| if (!st.isEmpty() && st.first() < smallestDay && st.first() <= lastDay) { | ||
| smallestDay = st.first(); | ||
| return Optional.of(st.first()); | ||
| } | ||
|
|
||
| return smallestDay == Integer.MAX_VALUE ? Optional.empty() : Optional.of(smallestDay); | ||
| } | ||
|
|
||
| private int findNearestWeekday(int day, int mon, int year) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [SHOULD] Please document the two invariants behind this lookup: a W adjustment moves at most two days, and the adjusted day is non-decreasing in the nominal day within a month. They justify the two-day look-back and returning the first result >= day, and should stay visible in future Quartz syncs. |
||
| Optional<Integer> nominal = findSmallestDay(day > 3 ? day - 2 : 1, mon, year, | ||
| nearestWeekdays); | ||
| while (nominal.isPresent()) { | ||
| int weekday = getNearestWeekday(nominal.get(), mon, year); | ||
| if (weekday >= day) { | ||
| return weekday; | ||
| } | ||
| nominal = findSmallestDay(nominal.get() + 1, mon, year, nearestWeekdays); | ||
| } | ||
| return -1; | ||
| } | ||
|
|
||
| private int getNearestWeekday(int day, int mon, int year) { | ||
| DayOfWeek dayOfWeek = LocalDate.of(year, mon, day).getDayOfWeek(); | ||
| if (dayOfWeek == DayOfWeek.SATURDAY) { | ||
| return day == 1 ? 3 : day - 1; | ||
| } | ||
| if (dayOfWeek == DayOfWeek.SUNDAY) { | ||
| return day == getLastDayOfMonth(mon, year) ? day - 2 : day + 1; | ||
| } | ||
| return day; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -149,8 +149,12 @@ node.discovery = { | |
| } | ||
|
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [NIT] Document that auto-stop does not trigger on SR self-produced blocks The stop-condition check now runs inside blockLock in TronNetDelegate.processBlock, but SR-produced blocks enter through BlockHandleImpl.produce -> manager.pushBlock and never pass through processBlock, so BlockHeight/BlockCount are not evaluated on a witness node's own-block path; a BlockTime hit likewise still needs the next processBlock'd block to arrive. The new comment block here explains UTC and missed slots but does not mention this boundary, so an SR operator configuring node.shutdown.BlockHeight can reasonably expect a stop that never comes. Suggestion: extend this comment (and the matching one in config.conf) to state that on witness/SR nodes auto-stop depends on network blocks arriving and self-produced blocks do not count; longer term, consider converging the stop evaluation at the Manager.pushBlock exit so one evaluation covers all block-ingestion paths (separate PR, as the PR follow-up notes). |
||
| # Custom stop condition | ||
| # BlockTime stops the node only at a block whose timestamp equals a cron time exactly (UTC). | ||
| # Mainnet produces a block every 3 s, so the seconds field must be a multiple of 3, and | ||
| # HH:00:03 / HH:00:06 UTC (HH = 00, 06, 12, 18) normally have no block (maintenance skip). | ||
| # If no block lands on that second (e.g. a missed slot), the node keeps running. | ||
| # node.shutdown = { | ||
| # BlockTime = "54 59 08 * * ?" # if block header time in persistent db matched | ||
| # BlockTime = "54 59 08 * * ?" # cron evaluated in UTC, if block header time in persistent db matched | ||
| # BlockHeight = 33350800 # if block header height in persistent db matched | ||
| # BlockCount = 12 # block sync count after node start | ||
| # } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,8 @@ | |
| import com.google.common.primitives.Longs; | ||
| import com.google.protobuf.ByteString; | ||
| import io.prometheus.client.Histogram; | ||
| import java.time.ZoneOffset; | ||
| import java.time.format.DateTimeFormatter; | ||
| import java.util.ArrayList; | ||
| import java.util.Arrays; | ||
| import java.util.Collection; | ||
|
|
@@ -24,7 +26,9 @@ | |
| import java.util.HashSet; | ||
| import java.util.LinkedList; | ||
| import java.util.List; | ||
| import java.util.Locale; | ||
| import java.util.Map; | ||
| import java.util.NoSuchElementException; | ||
| import java.util.Objects; | ||
| import java.util.Optional; | ||
| import java.util.Set; | ||
|
|
@@ -186,6 +190,9 @@ public class Manager { | |
| private static final int TX_ID_CACHE_SIZE = 100_000; | ||
| private static final int SLEEP_FOR_WAIT_LOCK = 10; | ||
| private static final int NO_BLOCK_WAITING_LOCK = 0; | ||
| private static final int AUTO_STOP_PREVIEW_COUNT = 3; | ||
| private static final DateTimeFormatter AUTO_STOP_TIME_FORMAT = DateTimeFormatter | ||
| .ofPattern("EEE yyyy-MM-dd HH:mm:ss 'UTC'", Locale.US).withZone(ZoneOffset.UTC); | ||
| private final int shieldedTransInPendingMaxCounts = | ||
| Args.getInstance().getShieldedTransInPendingMaxCounts(); | ||
| @Getter | ||
|
|
@@ -559,8 +566,8 @@ public void init() { | |
| // init auto-stop | ||
| try { | ||
| initAutoStop(); | ||
| } catch (IllegalArgumentException e) { | ||
| logger.error("Auto-stop params error: {}", e.getMessage()); | ||
| } catch (IllegalArgumentException | NoSuchElementException e) { | ||
| logger.error("Auto-stop params error: {}", e.toString()); | ||
| throw new TronError(e, TronError.ErrCode.AUTO_STOP_PARAMS); | ||
| } | ||
|
|
||
|
|
@@ -708,9 +715,16 @@ private void initAutoStop() { | |
| String.format("shutDownBlockCount %d is less than 1", exitCount)); | ||
| } | ||
|
|
||
| if (blockTime != null && blockTime.getNextValidTimeAfter(new Date(headTime)) == null) { | ||
| throw new IllegalArgumentException( | ||
| String.format("shutDownBlockTime %s is illegal", blockTime)); | ||
| if (blockTime != null) { | ||
| List<String> matches = nextCronMatches(blockTime, headTime); | ||
| if (matches.isEmpty()) { | ||
| throw new IllegalArgumentException( | ||
| String.format("shutDownBlockTime %s is illegal", blockTime)); | ||
| } | ||
| logger.info("Auto-stop: shutDownBlockTime {}, head block time {}, next cron matches {}. " | ||
| + "These are candidates: the node stops only if a block timestamp exactly matches " | ||
| + "one, and that is not guaranteed.", | ||
| blockTime, formatAutoStopTime(new Date(headTime)), matches); | ||
| } | ||
|
|
||
| if (exitHeight > 0 && exitCount > 0) { | ||
|
|
@@ -744,6 +758,23 @@ private void initAutoStop() { | |
| latestSolidityNumShutDown = CommonParameter.getInstance().getShutdownBlockHeight(); | ||
| } | ||
|
|
||
| private List<String> nextCronMatches(CronExpression cron, long headTime) { | ||
| List<String> matches = new ArrayList<>(AUTO_STOP_PREVIEW_COUNT); | ||
| Date next = new Date(headTime); | ||
| for (int i = 0; i < AUTO_STOP_PREVIEW_COUNT; i++) { | ||
| next = cron.getNextValidTimeAfter(next); | ||
| if (next == null) { | ||
| break; | ||
| } | ||
| matches.add(formatAutoStopTime(next)); | ||
| } | ||
| return matches; | ||
| } | ||
|
|
||
| private String formatAutoStopTime(Date time) { | ||
| return AUTO_STOP_TIME_FORMAT.format(time.toInstant()); | ||
| } | ||
|
|
||
| public AccountStore getAccountStore() { | ||
| return chainBaseManager.getAccountStore(); | ||
| } | ||
|
|
@@ -1045,11 +1076,13 @@ private void applyBlock(BlockCapsule block, List<TransactionCapsule> txs) | |
| updateFork(block); | ||
| if (System.currentTimeMillis() - block.getTimeStamp() >= 60_000) { | ||
| revokingStore.setMaxFlushCount(maxFlushCount); | ||
| if (Args.getInstance().getShutdownBlockTime() != null | ||
| && Args.getInstance().getShutdownBlockTime().getNextValidTimeAfter( | ||
| new Date(block.getTimeStamp() - maxFlushCount * 1000 * 3L)) | ||
| .compareTo(new Date(block.getTimeStamp())) <= 0) { | ||
| revokingStore.setMaxFlushCount(SnapshotManager.DEFAULT_MIN_FLUSH_COUNT); | ||
| CronExpression shutdownBlockTime = Args.getInstance().getShutdownBlockTime(); | ||
| if (shutdownBlockTime != null) { | ||
| Date nextStopTime = shutdownBlockTime.getNextValidTimeAfter( | ||
| new Date(block.getTimeStamp() - maxFlushCount * 1000 * 3L)); | ||
| if (nextStopTime != null && nextStopTime.compareTo(new Date(block.getTimeStamp())) <= 0) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [NIT] Documentation & test backlog (2 items rolled up) Grouped as one comment since both are doc-or-test asks rather than code defects.
|
||
| revokingStore.setMaxFlushCount(SnapshotManager.DEFAULT_MIN_FLUSH_COUNT); | ||
| } | ||
| } | ||
| if (latestSolidityNumShutDown > 0 && latestSolidityNumShutDown - block.getNum() | ||
| <= maxFlushCount) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[NIT] Track local Quartz backports with an in-file backport map
This vendored copy has drifted from upstream Quartz in at least three places: the W-lookup rewrite (findNearestWeekday/getNearestWeekday), the GregorianCalendar-based MAX_YEAR on this line, and the selective increment-message changes. The provenance and removal criteria for these local patches currently live only in commit messages, which become unreachable after squash/rebase/branch rewriting. A maintainer syncing a future upstream release could silently drop the local W/MAX_YEAR fixes, and without reachable removal criteria the local patches can never be retired.
Suggestion: add a short backport-map comment at the head of this file (after the license header) listing each local patch as upstream issue -> local change -> covering test -> removal condition (e.g. "remove once a quartz release newer than 2.5.2 contains the W-rewrite"), and file the promised upstream submission of the calendar/W/L-n/increment-message fixes as a trackable issue.