Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 37 additions & 44 deletions common/src/main/java/org/tron/common/cron/CronExpression.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Copy link
Copy Markdown
Collaborator

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.

private static final int SECOND = 0;
private static final int MINUTE = 1;
private static final int HOUR = 2;
Expand Down Expand Up @@ -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) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[QUESTION] Will the strict-parsing follow-up also cover L in the month and year fields? Both parse today: month L (0 0 0 1 L ?) throws NoSuchElementException on evaluation, while year L (0 0 0 1 * ? L) returns null and is rejected as having no future match. Non-blocking.

throw new ParseException("'L' not expected in seconds, minutes or hours fields.", i);
}
i++;
if (type == DAY_OF_WEEK) {
addToSet(7, 7, 0, type);
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The 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;
}
}
6 changes: 5 additions & 1 deletion common/src/main/resources/reference.conf
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,12 @@ node.discovery = {
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The 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
# }
Expand Down
6 changes: 5 additions & 1 deletion framework/src/main/java/org/tron/core/config/args/Args.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.text.ParseException;
import java.time.ZoneOffset;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand All @@ -28,6 +29,7 @@
import java.util.Map;
import java.util.Properties;
import java.util.Set;
import java.util.TimeZone;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.ConcurrentHashMap;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -676,7 +678,9 @@ private static void applyNodeConfig(NodeConfig nc) {
String shutdownBlockTime = nc.getShutdownBlockTime();
if (!shutdownBlockTime.isEmpty()) {
try {
PARAMETER.shutdownBlockTime = new CronExpression(shutdownBlockTime);
CronExpression blockTime = new CronExpression(shutdownBlockTime);
blockTime.setTimeZone(TimeZone.getTimeZone(ZoneOffset.UTC));
PARAMETER.shutdownBlockTime = blockTime;
} catch (ParseException e) {
throw new TronError(e, TronError.ErrCode.AUTO_STOP_PARAMS);
}
Expand Down
53 changes: 43 additions & 10 deletions framework/src/main/java/org/tron/core/db/Manager.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

  1. Backport inventory in the PR description. The description says "Selected Quartz 2.5.1 fixes are backported", but the change also includes the 2.5.2 findSmallestDay fix (test1420) and local-only W-lookup/MAX_YEAR patches, and the claim "Quartz 2.5.2 still fails these W cases" has no upstream issue anchor to audit. Suggestion: add a backport inventory table (upstream issue -> local change -> covering test -> upstream status) to the PR description.

  2. Deterministic coverage for the exhausted-cron null branch. The null check on nextStopTime in this block (the old NPE site fixed by this PR) is not exercised deterministically: the new BlockTimeStopTest cron uses a seconds field that is a multiple of 3, so a block always matches exactly and the null branch is only reached depending on flush timing. Suggestion: add a case with a seconds field that is not a multiple of 3 (e.g. 37) so no block timestamp can ever match and the null branch is guaranteed to be hit (the pre-fix code would NPE there).

revokingStore.setMaxFlushCount(SnapshotManager.DEFAULT_MIN_FLUSH_COUNT);
}
}
if (latestSolidityNumShutDown > 0 && latestSolidityNumShutDown - block.getNum()
<= maxFlushCount) {
Expand Down
30 changes: 15 additions & 15 deletions framework/src/main/java/org/tron/core/net/TronNetDelegate.java
Original file line number Diff line number Diff line change
Expand Up @@ -251,23 +251,23 @@ public void pushVerifiedBlock(BlockCapsule block) throws P2pException {
}

public void processBlock(BlockCapsule block, boolean isSync) throws P2pException {
if (!hitDown && dbManager.getLatestSolidityNumShutDown() > 0
&& dbManager.getLatestSolidityNumShutDown() == dbManager.getDynamicPropertiesStore()
.getLatestBlockHeaderNumberFromDB()) {

logger.info("Begin shutdown, currentBlockNum:{}, DbBlockNum:{}, solidifiedBlockNum:{}",
dbManager.getDynamicPropertiesStore().getLatestBlockHeaderNumber(),
dbManager.getDynamicPropertiesStore().getLatestBlockHeaderNumberFromDB(),
dbManager.getDynamicPropertiesStore().getLatestSolidifiedBlockNum());
hitDown = true;
LockSupport.unpark(hitThread);
return;
}
if (hitDown) {
return;
}
BlockId blockId = block.getBlockId();
synchronized (blockLock) {
if (!hitDown && dbManager.getLatestSolidityNumShutDown() > 0
&& dbManager.getLatestSolidityNumShutDown() == dbManager.getDynamicPropertiesStore()
.getLatestBlockHeaderNumberFromDB()) {

logger.info("Begin shutdown, currentBlockNum:{}, DbBlockNum:{}, solidifiedBlockNum:{}",
dbManager.getDynamicPropertiesStore().getLatestBlockHeaderNumber(),
dbManager.getDynamicPropertiesStore().getLatestBlockHeaderNumberFromDB(),
dbManager.getDynamicPropertiesStore().getLatestSolidifiedBlockNum());
hitDown = true;
LockSupport.unpark(hitThread);
return;
}
if (hitDown) {
return;
}
try {
if (freshBlockId.getIfPresent(blockId) == null) {
if (block.getNum() <= getHeadBlockId().getNum()) {
Expand Down
6 changes: 5 additions & 1 deletion framework/src/main/resources/config.conf
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,12 @@ node.discovery = {
}

# 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.
# }
Expand Down
Loading
Loading