liuhaoXD commented on a change in pull request #4247: fix thread unsafe problem
in server-alarm-plugin (#4230)
URL: https://github.com/apache/skywalking/pull/4247#discussion_r368285076
##########
File path:
oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RunningRule.java
##########
@@ -227,18 +221,17 @@ public void moveTo(LocalDateTime current) {
public void add(Metrics metrics) {
long bucket = metrics.getTimeBucket();
- LocalDateTime timebucket =
TIME_BUCKET_FORMATTER.parseLocalDateTime(bucket + "");
-
- int minutes = Minutes.minutesBetween(timebucket,
endTime).getMinutes();
- if (minutes == -1) {
- this.moveTo(timebucket);
-
- }
+ LocalDateTime timeBucket =
TIME_BUCKET_FORMATTER.parseLocalDateTime(bucket + "");
- lock.lock();
+ this.lock.lock();
try {
+ if (this.endTime == null) {
+ init();
+ this.endTime = timeBucket;
Review comment:
End time was not initialized.
hard-coded initialization of endTime is elegant, but may cause fatal problem
when the initial bucketTime is less than `1970-01-01T00:00:00Z`, under this
circumstance, `#add` will treated `window` as inited and invoke `#set` on
un-inited `this.values`, which will cause IOB Exception. Maybe an alternative
way is initializing endTime with value '0000-01-01T00:00:00Z‘, it's not so
elegant though..
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
