Fix race condition in rrdset_first_entry_t() and rrdset_last_entry_t() (#10276)
Markos Fountoulakis committed
Nov 28, 2020 at 15:53 UTC
5ffba490e381705750c4e2e3eeda47dee6b3cb3d
8 files changed
+69
-35
database/rrd.h
+44
-18
@@ -1027,16 +1027,15 @@ extern void rrdset_isnot_obsolete(RRDSET *st);
1027
#define rrdset_duration(st) ((time_t)( (((st)->counter >= ((unsigned long)(st)->entries))?(unsigned long)(st)->entries:(st)->counter) * (st)->update_every ))
1028
1029
// get the timestamp of the last entry in the round robin database
1030
-static inline time_t rrdset_last_entry_t(RRDSET *st) {
1030
+static inline time_t rrdset_last_entry_t_nolock(RRDSET *st)
1031
+{
1032
if (st->rrd_memory_mode == RRD_MEMORY_MODE_DBENGINE) {
1033
RRDDIM *rd;
1034
time_t last_entry_t = 0;
1035
1035
- int ret = netdata_rwlock_tryrdlock(&st->rrdset_rwlock);
1036
rrddim_foreach_read(rd, st) {
1037
last_entry_t = MAX(last_entry_t, rd->state->query_ops.latest_time(rd));
1038
}
1039
- if(0 == ret) netdata_rwlock_unlock(&st->rrdset_rwlock);
1039
1040
return last_entry_t;
1041
} else {
@@ -1044,25 +1043,46 @@ static inline time_t rrdset_last_entry_t(RRDSET *st) {
1043
}
1044
}
1045
1046
+static inline time_t rrdset_last_entry_t(RRDSET *st)
1047
+{
1048
+ time_t last_entry_t;
1049
+
1050
+ netdata_rwlock_rdlock(&st->rrdset_rwlock);
1051
+ last_entry_t = rrdset_last_entry_t_nolock(st);
1052
+ netdata_rwlock_unlock(&st->rrdset_rwlock);
1053
+
1054
+ return last_entry_t;
1055
+}
1056
+
1057
// get the timestamp of first entry in the round robin database
1048
-static inline time_t rrdset_first_entry_t(RRDSET *st) {
1058
+static inline time_t rrdset_first_entry_t_nolock(RRDSET *st)
1059
+{
1060
if (st->rrd_memory_mode == RRD_MEMORY_MODE_DBENGINE) {
1061
RRDDIM *rd;
1062
time_t first_entry_t = LONG_MAX;
1063
1053
- int ret = netdata_rwlock_tryrdlock(&st->rrdset_rwlock);
1064
rrddim_foreach_read(rd, st) {
1065
first_entry_t = MIN(first_entry_t, rd->state->query_ops.oldest_time(rd));
1066
}
1057
- if(0 == ret) netdata_rwlock_unlock(&st->rrdset_rwlock);
1067
1068
if (unlikely(LONG_MAX == first_entry_t)) return 0;
1069
return first_entry_t;
1070
} else {
1062
- return (time_t)(rrdset_last_entry_t(st) - rrdset_duration(st));
1071
+ return (time_t)(rrdset_last_entry_t_nolock(st) - rrdset_duration(st));
1072
}
1073
}
1074
1075
+static inline time_t rrdset_first_entry_t(RRDSET *st)
1076
+{
1077
+ time_t first_entry_t;
1078
+
1079
+ netdata_rwlock_rdlock(&st->rrdset_rwlock);
1080
+ first_entry_t = rrdset_first_entry_t_nolock(st);
1081
+ netdata_rwlock_unlock(&st->rrdset_rwlock);
1082
+
1083
+ return first_entry_t;
1084
+}
1085
+
1086
// get the timestamp of the last entry in the round robin database
1087
static inline time_t rrddim_last_entry_t(RRDDIM *rd) {
1088
if (rd->rrdset->rrd_memory_mode == RRD_MEMORY_MODE_DBENGINE)
@@ -1101,23 +1121,26 @@ static inline size_t rrdset_first_slot(RRDSET *st) {
1121
1122
// get the slot of the round robin database, for the given timestamp (t)
1123
// it always returns a valid slot, although may not be for the time requested if the time is outside the round robin database
1124
+// only valid when not using dbengine
1125
static inline size_t rrdset_time2slot(RRDSET *st, time_t t) {
1126
size_t ret = 0;
1127
+ time_t last_entry_t = rrdset_last_entry_t_nolock(st);
1128
+ time_t first_entry_t = rrdset_first_entry_t_nolock(st);
1129
1107
- if(t >= rrdset_last_entry_t(st)) {
1130
+ if(t >= last_entry_t) {
1131
// the requested time is after the last entry we have
1132
ret = rrdset_last_slot(st);
1133
}
1134
else {
1112
- if(t <= rrdset_first_entry_t(st)) {
1135
+ if(t <= first_entry_t) {
1136
// the requested time is before the first entry we have
1137
ret = rrdset_first_slot(st);
1138
}
1139
else {
1117
- if(rrdset_last_slot(st) >= ((rrdset_last_entry_t(st) - t) / (size_t)(st->update_every)))
1118
- ret = rrdset_last_slot(st) - ((rrdset_last_entry_t(st) - t) / (size_t)(st->update_every));
1140
+ if(rrdset_last_slot(st) >= ((last_entry_t - t) / (size_t)(st->update_every)))
1141
+ ret = rrdset_last_slot(st) - ((last_entry_t - t) / (size_t)(st->update_every));
1142
else
1120
- ret = rrdset_last_slot(st) - ((rrdset_last_entry_t(st) - t) / (size_t)(st->update_every)) + (unsigned long)st->entries;
1143
+ ret = rrdset_last_slot(st) - ((last_entry_t - t) / (size_t)(st->update_every)) + (unsigned long)st->entries;
1144
}
1145
}
1146
@@ -1130,8 +1153,11 @@ static inline size_t rrdset_time2slot(RRDSET *st, time_t t) {
1153
}
1154
1155
// get the timestamp of a specific slot in the round robin database
1156
+// only valid when not using dbengine
1157
static inline time_t rrdset_slot2time(RRDSET *st, size_t slot) {
1158
time_t ret;
1159
+ time_t last_entry_t = rrdset_last_entry_t_nolock(st);
1160
+ time_t first_entry_t = rrdset_first_entry_t_nolock(st);
1161
1162
if(slot >= (size_t)st->entries) {
1163
error("INTERNAL ERROR: caller of rrdset_slot2time() gives invalid slot %zu", slot);
@@ -1139,20 +1165,20 @@ static inline time_t rrdset_slot2time(RRDSET *st, size_t slot) {
1165
}
1166
1167
if(slot > rrdset_last_slot(st)) {
1142
- ret = rrdset_last_entry_t(st) - (size_t)st->update_every * (rrdset_last_slot(st) - slot + (size_t)st->entries);
1168
+ ret = last_entry_t - (size_t)st->update_every * (rrdset_last_slot(st) - slot + (size_t)st->entries);
1169
}
1170
else {
1145
- ret = rrdset_last_entry_t(st) - (size_t)st->update_every;
1171
+ ret = last_entry_t - (size_t)st->update_every;
1172
}
1173
1148
- if(unlikely(ret < rrdset_first_entry_t(st))) {
1174
+ if(unlikely(ret < first_entry_t)) {
1175
error("INTERNAL ERROR: rrdset_slot2time() on %s returns time too far in the past", st->name);
1150
- ret = rrdset_first_entry_t(st);
1176
+ ret = first_entry_t;
1177
}
1178
1153
- if(unlikely(ret > rrdset_last_entry_t(st))) {
1179
+ if(unlikely(ret > last_entry_t)) {
1180
error("INTERNAL ERROR: rrdset_slot2time() on %s returns time into the future", st->name);
1155
- ret = rrdset_last_entry_t(st);
1181
+ ret = last_entry_t;
1182
}
1183
1184
return ret;
database/rrddim.c
+2
-2
@@ -171,11 +171,11 @@ static void rrddim_query_finalize(struct rrddim_query_handle *handle) {
171
}
172
173
static time_t rrddim_query_latest_time(RRDDIM *rd) {
174
- return rrdset_last_entry_t(rd->rrdset);
174
+ return rrdset_last_entry_t_nolock(rd->rrdset);
175
}
176
177
static time_t rrddim_query_oldest_time(RRDDIM *rd) {
178
- return rrdset_first_entry_t(rd->rrdset);
178
+ return rrdset_first_entry_t_nolock(rd->rrdset);
179
}
180
181
health/health.c
+4
-2
@@ -501,8 +501,10 @@ static inline int rrdcalc_isrunnable(RRDCALC *rc, time_t now, time_t *next_run)
501
}
502
503
int update_every = rc->rrdset->update_every;
504
- time_t first = rrdset_first_entry_t(rc->rrdset);
505
- time_t last = rrdset_last_entry_t(rc->rrdset);
504
+ rrdset_rdlock(rc->rrdset);
505
+ time_t first = rrdset_first_entry_t_nolock(rc->rrdset);
506
+ time_t last = rrdset_last_entry_t_nolock(rc->rrdset);
507
+ rrdset_unlock(rc->rrdset);
508
509
if(unlikely(now + update_every < first /* || now - update_every > last */)) {
510
debug(D_HEALTH
web/api/exporters/shell/allmetrics_shell.c
+1
-1
@@ -119,7 +119,7 @@ void rrd_stats_api_v1_charts_allmetrics_json(RRDHOST *host, BUFFER *wb) {
119
, st->family
120
, st->context
121
, st->units
122
- , rrdset_last_entry_t(st)
122
+ , rrdset_last_entry_t_nolock(st)
123
);
124
125
chart_counter++;
web/api/formatters/json_wrapper.c
+4
-2
@@ -22,6 +22,7 @@ void rrdr_json_wrapper_begin(RRDR *r, BUFFER *wb, uint32_t format, RRDR_OPTIONS
22
sq[0] = '"';
23
}
24
25
+ rrdset_rdlock(r->st);
26
buffer_sprintf(wb, "{\n"
27
" %sapi%s: 1,\n"
28
" %sid%s: %s%s%s,\n"
@@ -38,11 +39,12 @@ void rrdr_json_wrapper_begin(RRDR *r, BUFFER *wb, uint32_t format, RRDR_OPTIONS
39
, kq, kq, sq, temp_rd?r->st->context:r->st->name, sq
40
, kq, kq, r->update_every
41
, kq, kq, r->st->update_every
41
- , kq, kq, (uint32_t)rrdset_first_entry_t(r->st)
42
- , kq, kq, (uint32_t)rrdset_last_entry_t(r->st)
42
+ , kq, kq, (uint32_t)rrdset_first_entry_t_nolock(r->st)
43
+ , kq, kq, (uint32_t)rrdset_last_entry_t_nolock(r->st)
44
, kq, kq, (uint32_t)r->before
45
, kq, kq, (uint32_t)r->after
46
, kq, kq);
47
+ rrdset_unlock(r->st);
48
49
for(c = 0, i = 0, rd = temp_rd?temp_rd:r->st->dimensions; rd && c < r->d ;c++, rd = rd->next) {
50
if(unlikely(r->od[c] & RRDR_DIMENSION_HIDDEN)) continue;
web/api/formatters/rrd2json.c
+3
-3
@@ -45,12 +45,12 @@ void build_context_param_list(struct context_param **param_list, RRDSET *st)
45
}
46
47
RRDDIM *rd1;
48
- (*param_list)->first_entry_t = MIN((*param_list)->first_entry_t, rrdset_first_entry_t(st));
49
- (*param_list)->last_entry_t = MAX((*param_list)->last_entry_t, rrdset_last_entry_t(st));
50
-
48
st->last_accessed_time = now_realtime_sec();
49
rrdset_rdlock(st);
50
51
+ (*param_list)->first_entry_t = MIN((*param_list)->first_entry_t, rrdset_first_entry_t_nolock(st));
52
+ (*param_list)->last_entry_t = MAX((*param_list)->last_entry_t, rrdset_last_entry_t_nolock(st));
53
+
54
rrddim_foreach_read(rd1, st) {
55
RRDDIM *rd = mallocz(rd1->memsize);
56
memcpy(rd, rd1, rd1->memsize);
web/api/formatters/rrdset2json.c
+2
-2
@@ -7,8 +7,8 @@
7
void rrdset2json(RRDSET *st, BUFFER *wb, size_t *dimensions_count, size_t *memory_used, int skip_volatile) {
8
rrdset_rdlock(st);
9
10
- time_t first_entry_t = rrdset_first_entry_t(st);
11
- time_t last_entry_t = rrdset_last_entry_t(st);
10
+ time_t first_entry_t = rrdset_first_entry_t_nolock(st);
11
+ time_t last_entry_t = rrdset_last_entry_t_nolock(st);
12
13
buffer_sprintf(wb,
14
"\t\t{\n"
web/api/queries/query.c
+9
-5
@@ -679,6 +679,7 @@ static void rrd2rrdr_log_request_response_metdata(RRDR *r
679
//, size_t before_slot
680
, const char *msg
681
) {
682
+ netdata_rwlock_rdlock(&r->st->rrdset_rwlock);
683
info("INTERNAL ERROR: rrd2rrdr() on %s update every %d with %s grouping %s (group: %ld, resampling_time: %ld, resampling_group: %ld), "
684
"after (got: %zu, want: %zu, req: %zu, db: %zu), "
685
"before (got: %zu, want: %zu, req: %zu, db: %zu), "
@@ -700,19 +701,19 @@ static void rrd2rrdr_log_request_response_metdata(RRDR *r
701
, (size_t)r->after
702
, (size_t)after_wanted
703
, (size_t)after_requested
703
- , (size_t)rrdset_first_entry_t(r->st)
704
+ , (size_t)rrdset_first_entry_t_nolock(r->st)
705
706
// before
707
, (size_t)r->before
708
, (size_t)before_wanted
709
, (size_t)before_requested
709
- , (size_t)rrdset_last_entry_t(r->st)
710
+ , (size_t)rrdset_last_entry_t_nolock(r->st)
711
712
// duration
713
, (size_t)(r->before - r->after + r->st->update_every)
714
, (size_t)(before_wanted - after_wanted + r->st->update_every)
715
, (size_t)(before_requested - after_requested)
715
- , (size_t)((rrdset_last_entry_t(r->st) - rrdset_first_entry_t(r->st)) + r->st->update_every)
716
+ , (size_t)((rrdset_last_entry_t_nolock(r->st) - rrdset_first_entry_t_nolock(r->st)) + r->st->update_every)
717
718
// slot
719
/*
@@ -730,6 +731,7 @@ static void rrd2rrdr_log_request_response_metdata(RRDR *r
731
// message
732
, msg
733
);
734
+ netdata_rwlock_unlock(&r->st->rrdset_rwlock);
735
}
736
#endif // NETDATA_INTERNAL_CHECKS
737
@@ -1575,8 +1577,10 @@ RRDR *rrd2rrdr(
1577
first_entry_t = context_param_list->first_entry_t;
1578
last_entry_t = context_param_list->last_entry_t;
1579
} else {
1578
- first_entry_t = rrdset_first_entry_t(st);
1579
- last_entry_t = rrdset_last_entry_t(st);
1580
+ rrdset_rdlock(st);
1581
+ first_entry_t = rrdset_first_entry_t_nolock(st);
1582
+ last_entry_t = rrdset_last_entry_t_nolock(st);
1583
+ rrdset_unlock(st);
1584
}
1585
1586
rrd_update_every = st->update_every;