Fossil SCM

Take care that a bug in the email alert sender results in missed alerts rather than an endless cascade of duplicate alerts.

drh 2018-08-09 14:15 trunk
Commit 164c3d1a6a16ff2f6fc490cb139a110a424f0461344850ef7726017f080e67f4
Parent 635d2f6317915ef…
1 file changed +45 -18
~ src/email.c
M src/email.c
+45 -18
--- src/email.c
+++ src/email.c
@@ -2145,10 +2145,17 @@
21452145
 ** which the subscriber has expressed interest and has
21462146
 ** appropriate privileges.
21472147
 **
21482148
 ** (4) Update the pending_alerts table to indicate that alerts have been
21492149
 ** sent.
2150
+**
2151
+** Update 2018-08-09: Do step (3) before step (4). Update the
2152
+** pending_alerts table *before* the emails are sent. That way, if
2153
+** the process malfunctions or crashes, some notifications may never
2154
+** be sent. But that is better than some recurring bug causing
2155
+** subscribers to be flooded with repeated notifications every 60
2156
+** seconds!
21502157
 */
21512158
 void email_send_alerts(u32 flags){
21522159
 EmailEvent *pEvents, *p;
21532160
 int nEvent = 0;
21542161
 Stmt q;
@@ -2160,11 +2167,10 @@
21602167
 const char *zDest = (flags & SENDALERT_STDOUT) ? "stdout" : 0;
21612168
 EmailSender *pSender = 0;
21622169
 u32 senderFlags = 0;
21632170
21642171
 if( g.fSqlTrace ) fossil_trace("-- BEGIN email_send_alerts(%u)\n", flags);
2165
- db_begin_transaction();
21662172
 email_schema(0);
21672173
 if( !email_enabled() ) goto send_alerts_done;
21682174
 zUrl = db_get("email-url",0);
21692175
 if( zUrl==0 ) goto send_alerts_done;
21702176
 zRepoName = db_get("email-subname",0);
@@ -2173,10 +2179,13 @@
21732179
 if( zFrom==0 ) goto send_alerts_done;
21742180
 if( flags & SENDALERT_TRACE ){
21752181
 senderFlags |= EMAIL_TRACE;
21762182
 }
21772183
 pSender = email_sender_new(zDest, senderFlags);
2184
+
2185
+ /* Step (1): Compute the alerts that need sending
2186
+ */
21782187
 db_multi_exec(
21792188
 "DROP TABLE IF EXISTS temp.wantalert;"
21802189
 "CREATE TEMP TABLE wantalert(eventId TEXT, needMod BOOLEAN, sentMod);"
21812190
 );
21822191
 if( flags & SENDALERT_DIGEST ){
@@ -2200,12 +2209,41 @@
22002209
 " FROM pending_alert"
22012210
 " WHERE sentSep IS FALSE;"
22022211
 "DELETE FROM wantalert WHERE needMod AND sentMod;"
22032212
 );
22042213
 }
2214
+
2215
+ /* Step 2: compute EmailEvent objects for every notification that
2216
+ ** needs sending.
2217
+ */
22052218
 pEvents = email_compute_event_text(&nEvent, (flags & SENDALERT_DIGEST)!=0);
22062219
 if( nEvent==0 ) goto send_alerts_done;
2220
+
2221
+ /* Step 4a: Update the pending_alerts table to designate the
2222
+ ** alerts as having all been sent. This is done *before* step (3)
2223
+ ** so that a crash will not cause alerts to be sent multiple times.
2224
+ ** Better a missed alert than being spammed with hundreds of alerts
2225
+ ** due to a bug.
2226
+ */
2227
+ if( (flags & SENDALERT_PRESERVE)==0 ){
2228
+ if( flags & SENDALERT_DIGEST ){
2229
+ db_multi_exec(
2230
+ "UPDATE pending_alert SET sentDigest=true"
2231
+ " WHERE eventid IN (SELECT eventid FROM wantalert);"
2232
+ );
2233
+ }else{
2234
+ db_multi_exec(
2235
+ "UPDATE pending_alert SET sentSep=true"
2236
+ " WHERE eventid IN (SELECT eventid FROM wantalert WHERE NOT needMod);"
2237
+ "UPDATE pending_alert SET sentMod=true"
2238
+ " WHERE eventid IN (SELECT eventid FROM wantalert WHERE needMod);"
2239
+ );
2240
+ }
2241
+ }
2242
+
2243
+ /* Step 3: Loop over subscribers. Send alerts
2244
+ */
22072245
 blob_init(&hdr, 0, 0);
22082246
 blob_init(&body, 0, 0);
22092247
 db_prepare(&q,
22102248
 "SELECT"
22112249
 " hex(subscriberCode)," /* 0 */
@@ -2288,27 +2326,16 @@
22882326
 }
22892327
 blob_reset(&hdr);
22902328
 blob_reset(&body);
22912329
 db_finalize(&q);
22922330
 email_free_eventlist(pEvents);
2293
- if( (flags & SENDALERT_PRESERVE)==0 ){
2294
- if( flags & SENDALERT_DIGEST ){
2295
- db_multi_exec(
2296
- "UPDATE pending_alert SET sentDigest=true"
2297
- " WHERE eventid IN (SELECT eventid FROM wantalert);"
2298
- "DELETE FROM pending_alert WHERE sentDigest AND sentSep;"
2299
- );
2300
- }else{
2301
- db_multi_exec(
2302
- "UPDATE pending_alert SET sentSep=true"
2303
- " WHERE eventid IN (SELECT eventid FROM wantalert WHERE NOT needMod);"
2304
- "UPDATE pending_alert SET sentMod=true"
2305
- " WHERE eventid IN (SELECT eventid FROM wantalert WHERE needMod);"
2306
- "DELETE FROM pending_alert WHERE sentDigest AND sentSep;"
2307
- );
2308
- }
2309
- }
2331
+
2332
+ /* Step 4b: Update the pending_alerts table to remove all of the
2333
+ ** alerts that have been completely sent.
2334
+ */
2335
+ db_multi_exec("DELETE FROM pending_alert WHERE sentDigest AND sentSep;");
2336
+
23102337
 send_alerts_done:
23112338
 email_sender_free(pSender);
23122339
 if( g.fSqlTrace ) fossil_trace("-- END email_send_alerts(%u)\n", flags);
23132340
 db_end_transaction(0);
23142341
 }
23152342
--- src/email.c
+++ src/email.c
@@ -2145,10 +2145,17 @@
2145 ** which the subscriber has expressed interest and has
2146 ** appropriate privileges.
2147 **
2148 ** (4) Update the pending_alerts table to indicate that alerts have been
2149 ** sent.
 
 
 
 
 
 
 
2150 */
2151 void email_send_alerts(u32 flags){
2152 EmailEvent *pEvents, *p;
2153 int nEvent = 0;
2154 Stmt q;
@@ -2160,11 +2167,10 @@
2160 const char *zDest = (flags & SENDALERT_STDOUT) ? "stdout" : 0;
2161 EmailSender *pSender = 0;
2162 u32 senderFlags = 0;
2163
2164 if( g.fSqlTrace ) fossil_trace("-- BEGIN email_send_alerts(%u)\n", flags);
2165 db_begin_transaction();
2166 email_schema(0);
2167 if( !email_enabled() ) goto send_alerts_done;
2168 zUrl = db_get("email-url",0);
2169 if( zUrl==0 ) goto send_alerts_done;
2170 zRepoName = db_get("email-subname",0);
@@ -2173,10 +2179,13 @@
2173 if( zFrom==0 ) goto send_alerts_done;
2174 if( flags & SENDALERT_TRACE ){
2175 senderFlags |= EMAIL_TRACE;
2176 }
2177 pSender = email_sender_new(zDest, senderFlags);
 
 
 
2178 db_multi_exec(
2179 "DROP TABLE IF EXISTS temp.wantalert;"
2180 "CREATE TEMP TABLE wantalert(eventId TEXT, needMod BOOLEAN, sentMod);"
2181 );
2182 if( flags & SENDALERT_DIGEST ){
@@ -2200,12 +2209,41 @@
2200 " FROM pending_alert"
2201 " WHERE sentSep IS FALSE;"
2202 "DELETE FROM wantalert WHERE needMod AND sentMod;"
2203 );
2204 }
 
 
 
 
2205 pEvents = email_compute_event_text(&nEvent, (flags & SENDALERT_DIGEST)!=0);
2206 if( nEvent==0 ) goto send_alerts_done;
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
2207 blob_init(&hdr, 0, 0);
2208 blob_init(&body, 0, 0);
2209 db_prepare(&q,
2210 "SELECT"
2211 " hex(subscriberCode)," /* 0 */
@@ -2288,27 +2326,16 @@
2288 }
2289 blob_reset(&hdr);
2290 blob_reset(&body);
2291 db_finalize(&q);
2292 email_free_eventlist(pEvents);
2293 if( (flags & SENDALERT_PRESERVE)==0 ){
2294 if( flags & SENDALERT_DIGEST ){
2295 db_multi_exec(
2296 "UPDATE pending_alert SET sentDigest=true"
2297 " WHERE eventid IN (SELECT eventid FROM wantalert);"
2298 "DELETE FROM pending_alert WHERE sentDigest AND sentSep;"
2299 );
2300 }else{
2301 db_multi_exec(
2302 "UPDATE pending_alert SET sentSep=true"
2303 " WHERE eventid IN (SELECT eventid FROM wantalert WHERE NOT needMod);"
2304 "UPDATE pending_alert SET sentMod=true"
2305 " WHERE eventid IN (SELECT eventid FROM wantalert WHERE needMod);"
2306 "DELETE FROM pending_alert WHERE sentDigest AND sentSep;"
2307 );
2308 }
2309 }
2310 send_alerts_done:
2311 email_sender_free(pSender);
2312 if( g.fSqlTrace ) fossil_trace("-- END email_send_alerts(%u)\n", flags);
2313 db_end_transaction(0);
2314 }
2315
--- src/email.c
+++ src/email.c
@@ -2145,10 +2145,17 @@
2145 ** which the subscriber has expressed interest and has
2146 ** appropriate privileges.
2147 **
2148 ** (4) Update the pending_alerts table to indicate that alerts have been
2149 ** sent.
2150 **
2151 ** Update 2018-08-09: Do step (3) before step (4). Update the
2152 ** pending_alerts table *before* the emails are sent. That way, if
2153 ** the process malfunctions or crashes, some notifications may never
2154 ** be sent. But that is better than some recurring bug causing
2155 ** subscribers to be flooded with repeated notifications every 60
2156 ** seconds!
2157 */
2158 void email_send_alerts(u32 flags){
2159 EmailEvent *pEvents, *p;
2160 int nEvent = 0;
2161 Stmt q;
@@ -2160,11 +2167,10 @@
2167 const char *zDest = (flags & SENDALERT_STDOUT) ? "stdout" : 0;
2168 EmailSender *pSender = 0;
2169 u32 senderFlags = 0;
2170
2171 if( g.fSqlTrace ) fossil_trace("-- BEGIN email_send_alerts(%u)\n", flags);
 
2172 email_schema(0);
2173 if( !email_enabled() ) goto send_alerts_done;
2174 zUrl = db_get("email-url",0);
2175 if( zUrl==0 ) goto send_alerts_done;
2176 zRepoName = db_get("email-subname",0);
@@ -2173,10 +2179,13 @@
2179 if( zFrom==0 ) goto send_alerts_done;
2180 if( flags & SENDALERT_TRACE ){
2181 senderFlags |= EMAIL_TRACE;
2182 }
2183 pSender = email_sender_new(zDest, senderFlags);
2184
2185 /* Step (1): Compute the alerts that need sending
2186 */
2187 db_multi_exec(
2188 "DROP TABLE IF EXISTS temp.wantalert;"
2189 "CREATE TEMP TABLE wantalert(eventId TEXT, needMod BOOLEAN, sentMod);"
2190 );
2191 if( flags & SENDALERT_DIGEST ){
@@ -2200,12 +2209,41 @@
2209 " FROM pending_alert"
2210 " WHERE sentSep IS FALSE;"
2211 "DELETE FROM wantalert WHERE needMod AND sentMod;"
2212 );
2213 }
2214
2215 /* Step 2: compute EmailEvent objects for every notification that
2216 ** needs sending.
2217 */
2218 pEvents = email_compute_event_text(&nEvent, (flags & SENDALERT_DIGEST)!=0);
2219 if( nEvent==0 ) goto send_alerts_done;
2220
2221 /* Step 4a: Update the pending_alerts table to designate the
2222 ** alerts as having all been sent. This is done *before* step (3)
2223 ** so that a crash will not cause alerts to be sent multiple times.
2224 ** Better a missed alert than being spammed with hundreds of alerts
2225 ** due to a bug.
2226 */
2227 if( (flags & SENDALERT_PRESERVE)==0 ){
2228 if( flags & SENDALERT_DIGEST ){
2229 db_multi_exec(
2230 "UPDATE pending_alert SET sentDigest=true"
2231 " WHERE eventid IN (SELECT eventid FROM wantalert);"
2232 );
2233 }else{
2234 db_multi_exec(
2235 "UPDATE pending_alert SET sentSep=true"
2236 " WHERE eventid IN (SELECT eventid FROM wantalert WHERE NOT needMod);"
2237 "UPDATE pending_alert SET sentMod=true"
2238 " WHERE eventid IN (SELECT eventid FROM wantalert WHERE needMod);"
2239 );
2240 }
2241 }
2242
2243 /* Step 3: Loop over subscribers. Send alerts
2244 */
2245 blob_init(&hdr, 0, 0);
2246 blob_init(&body, 0, 0);
2247 db_prepare(&q,
2248 "SELECT"
2249 " hex(subscriberCode)," /* 0 */
@@ -2288,27 +2326,16 @@
2326 }
2327 blob_reset(&hdr);
2328 blob_reset(&body);
2329 db_finalize(&q);
2330 email_free_eventlist(pEvents);
2331
2332 /* Step 4b: Update the pending_alerts table to remove all of the
2333 ** alerts that have been completely sent.
2334 */
2335 db_multi_exec("DELETE FROM pending_alert WHERE sentDigest AND sentSep;");
2336
 
 
 
 
 
 
 
 
 
 
 
2337 send_alerts_done:
2338 email_sender_free(pSender);
2339 if( g.fSqlTrace ) fossil_trace("-- END email_send_alerts(%u)\n", flags);
2340 db_end_transaction(0);
2341 }
2342

Keyboard Shortcuts

Open search /
Next entry (timeline) j
Previous entry (timeline) k
Open focused entry Enter
Show this help ?
Toggle theme Top nav button