Copilot commented on code in PR #13809:
URL: https://github.com/apache/cloudstack/pull/13809#discussion_r3733317127
##########
engine/schema/src/main/java/org/apache/cloudstack/backup/BackupVO.java:
##########
@@ -60,7 +60,7 @@ public class BackupVO implements Backup {
private String backupType;
@Column(name = "date")
- @Temporal(value = TemporalType.DATE)
+ @Temporal(value = TemporalType.TIMESTAMP)
private Date date;
Review Comment:
Changing `BackupVO.date` to `@Temporal(TIMESTAMP)` makes GenericDaoBase
bind/read this field via `setTimestamp`/`getTimestamp`, but the schema that
introduces the `cloud.backups` table defines `date` as `varchar(255)` (see
`engine/schema/src/main/resources/META-INF/db/schema-41310to41400.sql`).
Relying on implicit driver/DB conversions from TIMESTAMP <-> VARCHAR is
database/driver-specific and undermines the goal of typed temporal JDBC
handling.
Consider either (1) migrating the `cloud.backups.date` column to a proper
`DATETIME/TIMESTAMP` type (with an upgrade script + create schema update) or
(2) keeping this field as a string-backed column (and not using JDBC temporal
getters/setters for it).
##########
utils/src/main/java/com/cloud/utils/DateUtil.java:
##########
@@ -68,6 +71,19 @@ public static Date currentGMTTime() {
return new Date();
}
+ private static DateTimeFormatter getFormatter(String pattern, ZoneId zone)
{
+ String key = pattern + "|" + zone.getId();
+ DateTimeFormatter formatter = s_formatterCache.get(key);
+ if (formatter == null) {
+ formatter = DateTimeFormatter.ofPattern(pattern).withZone(zone);
+ DateTimeFormatter existing = s_formatterCache.putIfAbsent(key,
formatter);
+ if (existing != null) {
+ return existing;
+ }
+ }
+ return formatter;
+ }
Review Comment:
`getFormatter` manually does a `get` + `putIfAbsent`, which can still
allocate multiple `DateTimeFormatter` instances under contention and is more
verbose than needed. Using `ConcurrentHashMap.computeIfAbsent` keeps it atomic
and avoids redundant allocations while preserving the pattern+zone cache
behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
