-
Notifications
You must be signed in to change notification settings - Fork 141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(Forwarded from PttBug)「登入次數」的累計盲點 #104
Comments
實作登入次數自動增加機制的程式碼,根據以 PttBBS 原始碼為範圍、使用 ripgrep 所取得的字串搜尋結果,推測有兩處:
上述兩者在增加登入次數( 節錄自
|
time4_t base; | |
base = time4(0) - DAY_SECONDS; |
Lines 50 to 61 in 7b35778
if (urec.lastlogin >= base) | |
continue; | |
if (verbose) | |
fprintf(stderr, "update: %s (%s, %d) ->", urec.userid, | |
Cdatelite(&urec.lastlogin), urec.numlogindays); | |
/* user still online, let's mock it. */ | |
urec.lastlogin = now; | |
urec.numlogindays++; | |
if (verbose) | |
fprintf(stderr, "(%s, %d).\n", Cdatelite(&urec.lastlogin), urec.numlogindays); | |
passwd_update(last_uid, &urec); |
Line 77 in 7b35778
now = time(NULL); |
線上使用者登入後,其線上期間的計算方式為 now_cond - cuser.lastlogin
,其中 now_cond
為「判斷用目前時間值」,而 cuser
為對應此使用者的 userec_t
物件。
以下程式碼會阻止線上期間未超過一天的使用者的登入次數增加:
Lines 50 to 51 in 7b35778
if (urec.lastlogin >= base) | |
continue; |
urec.lastlogin >= base
… (1) →
urec.lastlogin >= now_cond - DAY_SECONDS
(now_cond
為「判斷用目前時間值」)→
urec.lastlogin + DAY_SECONDS >= now_cond
→
now_cond <= urec.lastlogin + DAY_SECONDS
… (2)→
now_cond - urec.lastlogin <= DAY_SECONDS
→
duration <= DAY_SECONDS
(duration
為線上期間)
可知在 update_online
所算出的線上期間大於一天的情況下,才會增加該使用者的登入次數。
作為比較,這是 commit 05702e5 時的對應程式碼:
Lines 45 to 46 in 05702e5
if (now < urec.lastlogin + DAY_SECONDS - 60 * 60) | |
continue; |
上述 (2) 式與之相比多了等號。
根據前面討論所提到的「線上期間的次數增加」機制的描述,以及 update_online
是支單獨程式的事實,我推測 update_online
是透過 cron
每天定時執行一次的。
假設電腦執行速度快得使「重設用目前時間值」(在上述節錄內容內,行號為 77 之行)與「判斷用目前時間值」(在上述節錄內容內,行號為 16 之行)皆相同。(因為參考 util/update_online.c
與其所使用的 common/bbs/cache.c
中的函式的定義,在程式執行流程中的上述兩行程式碼間不存在固定的延時,所以如果電腦執行速度相當快的話,這兩行可視為同時執行)
情況一
如果 cron
某次執行 update_online
時延遲,導致所取得的「重設用目前時間值」比預期延後超過一秒,這個情況下如果下次執行時所取得的時間值與預期相比沒有延後,那麼本次登入後已被 update_online
增加過登入次數且仍在線上的使用者,將會因為所算出的線上期間小於一天,所以其登入次數不會增加,造成少算的狀況。
一種解決方法是重新引入時間判斷的誤差容許值,但在定時執行未出現延遲時會導致允許將線上期間未超過一天的使用者的登入次數增加。
另一種解決方法是將「重設用目前時間值」與「判斷用目前時間值」取為最近的分鐘或小時的開始時間。
又另一種解決方法請見情況四。
假設電腦執行速度快,cron
每天皆一秒不差地定時執行一次 update_online
,使得其執行時間的值、「重設用目前時間值」、與「判斷用目前時間值」皆相同。
情況二
如果使用者被過去最近一次 update_online
的每天定時執行增加了登入次數,或如果使用者本次登入後未被 update_online
增加過登入次數,並且使用者登入的時間點為過去最近一次 update_online
執行時所取得的「判斷用目前時間值」的時間點,那麼下一次執行時,這個使用者的線上期間將會等於一天,所以其登入次數不會增加,造成少算的狀況。遇到下二次執行時才會增加,但僅增加一次。
修正方法是移除上述 (1) 式中的等號。
情況三
如果使用者本次登入後未被 update_online
增加過登入次數,並且使用者登入的時間點比過去最近一次 update_online
執行時所取得的「判斷用目前時間值」的時間點還晚超過一秒後登入,那麼這個使用者的線上期間達到大於等於一天的時間會落在下一次到下二次遇到 update_online
每天定時執行之間。這個情況下,如果這個使用者在下二次遇到 update_online
的每天定時執行前離線,將會因為 update_online
不處理離線使用者而不增加其登入次數而造成少算的狀況。
一種解決方法是在使用者登出時會執行的處理函式(例如 pwcuExitSave()
)中,加上以下處理邏輯:
如果以
cuser.lastlogin - now
算出的線上期間大於等於一天則增加一次登入次數(「離線時的次數增加」)。
但在使用者的對應處理程序異常終止執行而導致「離線時的次數增加」機制未被執行的情況下,還是會造成少算的狀況。
情況四
以下是虛構的說明範例:
- 使用者甲在某天 03:59:59 登入,在後天 04:00:01 登出,在線上共 48 時 0 分 2 秒
- 登入次數增加時間點:第一天 03:59:59、第二天 04:00:00、第三天 04:00:00,共三次
- 使用者乙在某天 04:00:01 登入,在後天 04:00:03 登出,在線上共 48 時 0 分 2 秒
- 登入次數增加時間點:第一天 04:00:01、第三天 04:00:00,共兩次
可見使用者甲與乙的線上期間相同,僅有登入與登出的時間有些許前後差別,但乙的登入次數卻少算了一次。
一種解決方法是將「重設用目前時間值」取為 cuser.lastlogin + DAY_SECONDS
以保留實際線上期間的資訊,並搭配使用情況三的解決方法處理離線時的線上期間。
使用此解決方法後的上述說明範例:
- 使用者甲在某天 03:59:59 登入,在後天 04:00:01 登出,在線上共 48 時 0 分 2 秒
- 登入次數增加時間點:第一天 03:59:59(登入)、第二天 04:00:00(線上;尚有 1 秒未計)、第三天 04:00:00(線上;尚有 1 秒未計),共三次
- 使用者乙在某天 04:00:01 登入,在後天 04:00:03 登出,在線上共 48 時 0 分 2 秒
- 登入次數增加時間點:第一天 04:00:01(登入)、第三天 04:00:00(線上;尚有 23 時 59 分 59 秒未計)、第三天 04:00:03(登出;尚有 2 秒未計),共三次
Legend: * Event: Name of event ("Login", "UpdateDN" (by update_online), and "Logout") * UpdateDN: Update on day N (day 1 is the day of login) * UpdateDN is assumed to usually occur at 04:00:00 each day * If Login and UpdateD1 occur at the same time, UpdateD1 is omitted from the chart for clarity * Time: Time of event * TDur: Total online duration since login * CTime/NTime: Checkpoint (lastlogin) set by the current method/this new method * CDurB/A: Online duration before/after the checkpoint is set (current method) * NDurB/A: Online duration before/after the checkpoint is set (new method) * CIncr/NIncr: Increment of numlogindays (current/new method) --- Case: Online duration after last checkpoint = 1 day (86400 seconds) The login count does not increase when the online duration = 1 day due to a regression introduced in 05bb3eb, where >= instead of > was used to filter out users whose last checkpoint time (lastlogin) were later than 1 day before now. Event: Login UpdateD2 UpdateD3 UpdateD4 UpdateD5 Time: 04:00:00 28:00:00 52:00:00 76:00:00 100:00:00 TDur: 00h00m00 24h00m00 48h00m00 72h00m00 96h00m00 CTime: 04:00:00 -- 52:00:00 -- 100:00:00 CDurB: 00h00m00 24h00m00 48h00m00 24h00m00 48h00m00 CDurA: 00h00m00 -- 00h00m00 -- 00h00m00 CIncr: +1 0 +1 0 +1 Total: +3 NTime: 04:00:00 28:00:00 52:00:00 76:00:00 100:00:00 NDurB: 00h00m00 24h00m00 24h00m00 24h00m00 24h00m00 NDurA: 00h00m00 00h00m00 00h00m00 00h00m00 00h00m00 NIncr: +1 +1 +1 +1 +1 Total: +5 This caused the login count to increases only by 1 per 2 days for users who never logout. This regression is fixed by using > for filter out users to allow users with online duration = 1 day to have their login count increased. For the rest cases, assume this regression is fixed for the current method. --- Case: The launch of update_online by cron lagged or even failed In this fictitious case, the launch of update_online lags for 1 seconds on day 2 and fails on day 5. Event: Login UpdateD2 UpdateD3 UpdateD4 UpdateD6 Time: 04:00:00 28:00:01 52:00:00 76:00:00 124:00:00 TDur: 00h00m00 24h00m01 48h00m00 72h00m00 120h00m00 CTime: 04:00:00 28:00:01 -- 76:00:00 124:00:00 CDurB: 00h00m00 24h00m01 23h59m59 47h59m59 48h00m00 CDurA: 00h00m00 00h00m00 -- 00h00m00 00h00m00 CIncr: +1 +1 0 +1 +1 Total: +4 NTime: 04:00:00 28:00:00 52:00:00 76:00:00 124:00:00 NDurB: 00h00m00 24h00m01 24h00m00 24h00m00 48h00m00 NDurA: 00h00m00 00h00m01 00h00m00 00h00m00 00h00m00 NIncr: +1 +1 +1 +1 +2 Total: +6 --- Case: Same total online duration, different login count increment Compare the following cases. Event: Login UpdateD2 UpdateD3 Logout Time: 04:00:01 28:00:00 52:00:00 52:00:03 TDur: 00h00m00 23h59m59 47h59m59 48h00m02 CTime: 04:00:01 -- 52:00:00 -- CDurB: 00h00m00 23h59m59 47h59m59 00h00m03 CDurA: 00h00m00 -- 00h00m00 -- CIncr: +1 0 +1 0 Total: +2 NTime: 04:00:01 -- 28:00:01 52:00:03 NDurB: 00h00m00 23h59m59 47h59m59 24h00m02 NDurA: 00h00m00 00h00m00 23h59m59 00h00m02 NIncr: +1 0 +1 +1 Total: +3 Event: Login UpdateD1 UpdateD2 UpdateD3 Logout Time: 03:59:59 04:00:00 28:00:00 52:00:00 52:00:01 TDur: 00h00m00 00h00m01 24h00m01 48h00m01 48h00m02 CTime: 03:59:59 -- 52:00:00 -- -- CDurB: 00h00m00 00h00m01 24h00m01 24h00m00 00h00m01 CDurA: 00h00m00 -- 00h00m00 00h00m00 -- CIncr: +1 0 +1 +1 0 Total: +3 NTime: 03:59:59 -- 27:59:59 51:59:59 -- NDurB: 00h00m00 00h00m01 24h00m01 24h00m01 00h00m02 NDurA: 00h00m00 -- 00h00m01 00h00m01 -- NIncr: +1 0 +1 +1 0 Total: +3 --- * mbbsd/passwd.c * util/update_online.c
The method for increasing user's login count has been refined to prevent undercounting of the login times. It works as follows: * Login time increment at login (the same as the current method) For users who haven't logged in today, increase their login count by 1 at login. * Login time increments according to the online duration When either update_online runs *or the user logs out*, if the user's online duration reaches N days (>= N⋅86400 seconds), increase the user's login count by N and the user's lastlogin by N days. (Increasing lastlogin by N days makes the online duration decrease by N days) Compare it to the current method: * The online duration is not checked at logout -> is checked at logout * Increment is performed only when the online duration *exceeds* 1 day (> 86400 seconds). -> is performed also when the duration reaches 1 day (= 86400 seconds) * Explained below as the login count increment rate issue * The login count only increases by 1 -> can increase by N * lastlogin is set to the current time (resets the online duration to 0) -> lastlogin is increased by N days (reduces the online duration by N days) * A reset would cause users to lose some of their online duration, enough to cause undercounting. * The reset method is also prone to unpredictable cron job delays. Executing update_online is no longer necessary to increase users' login count but is still needed to minimize users' loss of login times due to abnormal termination of their session. --- The login count increment rate issue resulted from the slightly flawed condition for excluding users from increment. The condition should exclude users whose online duration < 1 day. When update_online was introduced on 2014-03-25 (UTC+8), this condition was wrongly flipped as `u->lastlogin + DAY_SECONDS <= now`. It was corrected within an hour to be `now < urec.lastlogin + DAY_SECONDS` (equivalently `urec.lastlogin + DAY_SECONDS > now`). See commits 4f69140 ("Add utility to update user online counter (lastlogin/numlogindays).") and 6871fbb ("Update using ulist.") for details. A 1-hour tolerance of online duration was soon introduced and cancelled. See commit 05702e5 ("update_online: prevent too verbose messages and allow some tolerance in duration."). Within 1 day, another commit changed this condition to `urec.lastlogin >= base`, where `base` was defined as 09:40 of the current day (in the local timezone). It may have meant to increase the login count for users logged in before 09:40. This was stated to be inspired by the login time increment at login. However, update_online seems to have run at 00:00 of the day at that time. This made the condition always false since 09:40 would be a time in the future, and thus all online users' login count were increased unconditionally. This caused overcounting of the login times, as users might manage to log in at 00:00 and before update_online ran, thus getting 2 login times on that day. (Cron jobs may delay for seconds, especially when the system load is high) See commit 05bb3eb ("Change update_online to do same way as pwcu does.") for details. In order to address this issue, commits f1328b3 ("Fix calculation error in update_online.", 2014-12-23 (UTC+8)) and d38b21d ("Fix update_online again, to avoid timezone problems.", 2014-12-24 (UTC+8)) were made to redefine `base` as `time(0) - DAY_SECONDS`. The relevant logic somehow reverted to using the logic in commit 6871fbb ("Update using ulist."). However, the condition `urec.lastlogin >= base` did not revert accordingly and became arithmetically equivalent to `urec.lastlogin + DAY_SECONDS >= now`, which does exclude users whose online duration = 1 day. Since update_online runs every 1 day in the current system configuration, the users' online duration between 2 successive runs cannot exceed 1 day (assume that cron jobs have the same delay in every run). In other words, when the users' lastlogin was reset in an update_online run, their login count would not increase in the next update_online run, which made their login count only increase by 1 every 2 days. This issue is fixed by changing this condition to `urec.lastlogin > base` to allow users with online duration = 1 day to have their login count increased. --- * mbbsd/passwd.c: * pwcuExitSave(): * Check the user's online duration at logout * If the duration reaches N days, increase the login count by N and lastlogin by N days * util/update_online.c: * fastcheck(): * Do not exclude users whose online duration = 1 day from login count increment * If the duration reaches N days, increase the login count by N and lastlogin by N days * Introduce variable `days` for the reached number of days of the online duration * main(): * Drop the assignment of now-unused variable `now`
The method for increasing user's login count has been refined to prevent undercounting of the login times. It works as follows: * Login time increment at login (the same as the current method) For users who haven't logged in today, increase their login count by 1 at login. * Login time increments according to the online duration When either update_online runs *or the user logs out*, if the user's online duration reaches N days (>= N⋅86400 seconds), increase the user's login count by N and the user's lastlogin by N days. (Increasing lastlogin by N days makes the online duration decrease by N days) Compare it to the current method: * The online duration is not checked at logout -> is checked at logout * Increment is performed only when the online duration *exceeds* 1 day (> 86400 seconds). -> is performed also when the duration reaches 1 day (= 86400 seconds) * Explained below as the login count increment rate issue * The login count only increases by 1 -> can increase by N * lastlogin is set to the current time (resets the online duration to 0) -> lastlogin is increased by N days (reduces the online duration by N days) * A reset would cause users to lose some of their online duration, enough to cause undercounting. * The reset method is also prone to unpredictable cron job delays. Executing update_online is no longer necessary to increase users' login count but is still needed to minimize users' loss of login times due to abnormal termination of their session. --- The login count increment rate issue resulted from the slightly flawed condition for excluding users from increment. The condition should exclude users whose online duration < 1 day. When update_online was introduced on 2014-03-25 (UTC+8), this condition was wrongly flipped as `u->lastlogin + DAY_SECONDS <= now`. It was corrected within an hour to be `now < urec.lastlogin + DAY_SECONDS` (equivalently `urec.lastlogin + DAY_SECONDS > now`). See commits 4f69140 ("Add utility to update user online counter (lastlogin/numlogindays).") and 6871fbb ("Update using ulist.") for details. A 1-hour tolerance of online duration was soon introduced and cancelled. See commit 05702e5 ("update_online: prevent too verbose messages and allow some tolerance in duration."). Within 1 day, another commit changed this condition to `urec.lastlogin >= base`, where `base` was defined as 09:40 of the current day (in the local time zone). It may have meant to increase the login count for users logged in before 09:40. This was stated to be inspired by the login time increment at login. However, update_online seems to have run at 00:00 of the day at that time. This made the condition always false since 09:40 would be a time in the future, and thus all online users' login count were increased unconditionally. This caused overcounting of the login times, as users might manage to log in at 00:00 and before update_online ran, thus getting 2 login times on that day. (Cron jobs may delay for seconds, especially when the system load is high) See commit 05bb3eb ("Change update_online to do same way as pwcu does.") for details. In order to address this issue, commits f1328b3 ("Fix calculation error in update_online.", 2014-12-23 (UTC+8)) and d38b21d ("Fix update_online again, to avoid timezone problems.", 2014-12-24 (UTC+8)) were made to redefine `base` as `time(0) - DAY_SECONDS`. The relevant logic somehow reverted to using the logic in commit 6871fbb ("Update using ulist."). However, the condition `urec.lastlogin >= base` did not revert accordingly and became arithmetically equivalent to `urec.lastlogin + DAY_SECONDS >= now`, which does exclude users whose online duration = 1 day. Since update_online runs every 1 day in the current system configuration, the users' online duration between 2 successive runs cannot exceed 1 day (assume that cron jobs have the same delay in every run). In other words, when the users' lastlogin was reset in an update_online run, their login count would not increase in the next update_online run, which made their login count only increase by 1 every 2 days. This issue is fixed by changing this condition to `urec.lastlogin > base` to allow users with online duration = 1 day to have their login count increased. --- * mbbsd/passwd.c: * pwcuExitSave(): * Check the user's online duration at logout * If the duration reaches N days, increase the login count by N and lastlogin by N days * util/update_online.c: * fastcheck(): * Do not exclude users whose online duration = 1 day from login count increment * If the duration reaches N days, increase the login count by N and lastlogin by N days * Introduce variable `days` for the reached number of days of the online duration * main(): * Drop the assignment of now-unused variable `now`
The method for increasing user's login count has been refined to prevent undercounting of the login times. It works as follows: * Login time increment at login (the same as the current method) For users who haven't logged in today, increase their login count by 1 at login. * Login time increments according to the online duration When either update_online runs *or the user logs out*, if the user's online duration reaches N days (>= N⋅86400 seconds), increase the user's login count by N and the user's lastlogin by N days. (Increasing lastlogin by N days makes the online duration decrease by N days) Compare it to the current method: * The online duration is not checked at logout -> is checked at logout * Increment is performed only when the online duration *exceeds* 1 day (> 86400 seconds) -> is performed also when the duration reaches 1 day (= 86400 seconds) * Explained below as the login count increment rate issue * The login count only increases by 1 -> can increase by N * lastlogin is set to the current time (resets the online duration to 0) -> lastlogin is increased by N days (reduces the online duration by N days) * A reset would cause users to lose some of their online duration, enough to cause undercounting. * The reset method is also prone to unpredictable cron job delays. Executing update_online is no longer necessary to increase users' login count but is still needed to minimize users' loss of login times due to abnormal termination of their session. --- The login count increment rate issue resulted from the slightly flawed condition for excluding users from increment. The condition should exclude users whose online duration < 1 day. When update_online was introduced on 2014-03-25 (UTC+8), this condition was wrongly flipped as `u->lastlogin + DAY_SECONDS <= now`. It was corrected within an hour to be `now < urec.lastlogin + DAY_SECONDS` (equivalently `urec.lastlogin + DAY_SECONDS > now`). See commits 4f69140 ("Add utility to update user online counter (lastlogin/numlogindays).") and 6871fbb ("Update using ulist.") for details. A 1-hour tolerance of online duration was soon introduced and cancelled. See commit 05702e5 ("update_online: prevent too verbose messages and allow some tolerance in duration."). Within 1 day, another commit changed this condition to `urec.lastlogin >= base`, where `base` was defined as 09:40 of the current day (in the local time zone). It may have meant to increase the login count for users logged in before 09:40. This was stated to be inspired by the login time increment at login. However, update_online seems to have run at 00:00 of the day at that time. This made the condition always false since 09:40 would be a time in the future, and thus all online users' login count were increased unconditionally. This caused overcounting of the login times, as users might manage to log in at 00:00 and before update_online ran, thus getting 2 login times on that day. (Cron jobs may delay for seconds, especially when the system load is high) See commit 05bb3eb ("Change update_online to do same way as pwcu does.") for details. In order to address this issue, commits f1328b3 ("Fix calculation error in update_online.", 2014-12-23 (UTC+8)) and d38b21d ("Fix update_online again, to avoid timezone problems.", 2014-12-24 (UTC+8)) were made to redefine `base` as `time(0) - DAY_SECONDS`. The relevant logic somehow reverted to using the logic in commit 6871fbb ("Update using ulist."). However, the condition `urec.lastlogin >= base` did not revert accordingly and became arithmetically equivalent to `urec.lastlogin + DAY_SECONDS >= now`, which does exclude users whose online duration = 1 day. Since update_online runs every 1 day in the current system configuration, the users' online duration between 2 successive runs cannot exceed 1 day (assume that cron jobs have the same delay in every run). In other words, when the users' lastlogin was reset in an update_online run, their login count would not increase in the next update_online run, which made their login count only increase by 1 every 2 days. This issue is fixed by changing this condition to `urec.lastlogin > base` to allow users with online duration = 1 day to have their login count increased. --- * mbbsd/passwd.c: * pwcuExitSave(): * Check the user's online duration at logout * If the duration reaches N days, increase the login count by N and lastlogin by N days * util/update_online.c: * fastcheck(): * Do not exclude users whose online duration = 1 day from login count increment * If the duration reaches N days, increase the login count by N and lastlogin by N days * Introduce variable `days` for the reached number of days of the online duration * main(): * Drop the assignment of now-unused variable `now`
…line users [ptt#104] The algorithm for increasing online users' login count has been improved, which fixes the issue that the increment was sometimes done only per 2 days. When update_online was introduced in 4f69140 ("Add utility to update user online counter (lastlogin/numlogindays).", 2014-03-25 (UTC+8)), the condition for excluding users from an update by their online duration was wrongly flipped as `u->lastlogin + DAY_SECONDS <= now` but had been corrected within 1 hour into `now < urec.lastlogin + DAY_SECONDS` (equivalently `urec.lastlogin + DAY_SECONDS > now`) in 6871fbb ("Update using ulist."). A 1-hour tolerance of online duration was soon introduced in 05702e5 ("update_online: prevent too verbose messages and allow some tolerance in duration.") but had soon been cancelled. Within 1 day, 05bb3eb ("Change update_online to do same way as pwcu does.") changed this condition into `urec.lastlogin >= base`, where `base` was 09:40 of the current day (in the local time zone). However, update_online seems to have run at 00:00 every day for PTT, which made the condition always false, and thus all online users' login count were increased unconditionally. This had allowed the users to earn 2 login count within a day by logging in at 00:00 and before update_online ran. (Note that Cron can lags due to a high system load) Therefore, f1328b3 ("Fix calculation error in update_online.", 2014-12-23 (UTC+8)) and d38b21d ("Fix update_online again, to avoid timezone problems.", 2014-12-24 (UTC+8)) redefined `base` as `time(0) - DAY_SECONDS`, which has partially reverted the logic to 6871fbb ("Update using ulist."). However, the condition `urec.lastlogin >= base` was not reverted accordingly and has become equivalent to `urec.lastlogin + DAY_SECONDS >= now`, which does exclude users whose online duration = 1 day. This has caused online users' login count increase only by 1 per 2 days (assume that Cron did not lag). In the new algorithm, users' increment of login count is first calculated, then users whose login count do not increase are excluded, and then their login time and count are advanced/increased accordingly. It works regardless of the time and frequency that update_online runs. To account for the possible lagging of Cron, the reference time is now `now`, the nearest minute in the past from now. Because updating users' login count at pwcuExitSave() would cause unwanted logs, perform such updating at logout is not considered. Instead, to minimize online user's possible loss of login count, update_online can be excuted multiple times per day. * fastcheck() * add local `now` for the nearest minute in the past * add local `days` for the count of days since `urec.lastlogin` until `now` * make `urec.lastlogin` advance by `days * DAY_SECONDS` instead of reset to `now` * make `urec.numlogindays` increase by `days` instead of fixed 1 * main() * drop the assignment of now-unused global `now`
…line users [ptt#104] The algorithm for increasing online users' login count has been improved, which fixes the issue that the increment was sometimes done only per 2 days. When update_online was introduced in 4f69140 ("Add utility to update user online counter (lastlogin/numlogindays).", 2014-03-25 (UTC+8)), the condition for excluding users from an update by their online duration was wrongly flipped as `u->lastlogin + DAY_SECONDS <= now` but had been corrected within 1 hour into `now < urec.lastlogin + DAY_SECONDS` (equivalently `urec.lastlogin + DAY_SECONDS > now`) in 6871fbb ("Update using ulist."). A 1-hour tolerance of online duration was soon introduced in 05702e5 ("update_online: prevent too verbose messages and allow some tolerance in duration.") but had soon been cancelled. Within 1 day, 05bb3eb ("Change update_online to do same way as pwcu does.") changed this condition into `urec.lastlogin >= base`, where `base` was 09:40 of the current day (in the local time zone). However, update_online seems to have run at 00:00 every day for PTT, which made the condition always false, and thus all online users' login count were increased unconditionally. This had allowed the users to earn a login count of 2 within a day by logging in at 00:00 and before update_online ran. (Note that Cron can lags due to a high system load) Therefore, f1328b3 ("Fix calculation error in update_online.", 2014-12-23 (UTC+8)) and d38b21d ("Fix update_online again, to avoid timezone problems.", 2014-12-24 (UTC+8)) redefined `base` as `time(0) - DAY_SECONDS`, which has partially reverted the logic to 6871fbb ("Update using ulist."). However, the condition `urec.lastlogin >= base` was not reverted accordingly and has become equivalent to `urec.lastlogin + DAY_SECONDS >= now`, which does exclude users whose online duration = 1 day. This has caused online users' login count increase only by 1 per 2 days (assume that Cron did not lag). In the new algorithm, users' increment of login count is first calculated, then users whose login count do not increase are excluded, and then their login time and count are advanced/increased accordingly. It works regardless of the time and frequency that update_online runs. To account for the possible lagging of Cron, the reference time is now `now`, the nearest minute in the past from now. Because updating users' login count at pwcuExitSave() would cause unwanted logs, perform such updating at logout is not considered. Instead, to minimize online user's possible loss of login count, update_online can be excuted multiple times per day. * fastcheck() * add local `now` for the nearest minute in the past * add local `days` for the count of days since `urec.lastlogin` until `now` * make `urec.lastlogin` advance by `days * DAY_SECONDS` instead of reset to `now` * make `urec.numlogindays` increase by `days` instead of fixed 1 * main() * drop the assignment of now-unused global `now`
…line users [ptt#104] The algorithm for increasing online users' login count has been improved, which fixes the issue that the increment was sometimes done only per 2 days. When update_online was introduced in 4f69140 ("Add utility to update user online counter (lastlogin/numlogindays).", 2014-03-25 (UTC+8)), the condition for excluding users from an update by their online duration was wrongly flipped as `u->lastlogin + DAY_SECONDS <= now` but had been corrected within 1 hour into `now < urec.lastlogin + DAY_SECONDS` (equivalently `urec.lastlogin + DAY_SECONDS > now`) in 6871fbb ("Update using ulist."). A 1-hour tolerance of online duration was soon introduced in 05702e5 ("update_online: prevent too verbose messages and allow some tolerance in duration.") but had soon been cancelled. Within 1 day, 05bb3eb ("Change update_online to do same way as pwcu does.") changed this condition into `urec.lastlogin >= base`, where `base` was 09:40 of the current day (in the local time zone). However, update_online seems to have run at 00:00 every day for PTT, which made the condition always false, and thus all online users' login count were increased unconditionally. This had allowed the users to earn a login count of 2 within a day by logging in at 00:00 and before update_online ran. (Note that Cron can lags due to a high system load) Therefore, f1328b3 ("Fix calculation error in update_online.", 2014-12-23 (UTC+8)) and d38b21d ("Fix update_online again, to avoid timezone problems.", 2014-12-24 (UTC+8)) redefined `base` as `time(0) - DAY_SECONDS`, which had partially reverted the logic to 6871fbb ("Update using ulist."). However, the condition `urec.lastlogin >= base` was not reverted accordingly and had become equivalent to `urec.lastlogin + DAY_SECONDS >= now`, which does exclude users whose online duration = 1 day. This had caused online users' login count increase only by 1 per 2 days (assume that Cron did not lag). In the new algorithm, users' increment of login count is first calculated, then users whose login count do not increase are excluded, and then their login time and count are advanced/increased accordingly. It works regardless of the time and frequency that update_online runs. To account for the possible lagging of Cron, the reference time is now `now`, the nearest minute in the past from now. Because updating users' login count at pwcuExitSave() would cause unwanted logs, perform such updating at logout is not considered. Instead, to minimize online user's possible loss of login count, update_online can be excuted multiple times per day. * fastcheck() * add local `now` for the nearest minute in the past * add local `days` for the count of days since `urec.lastlogin` until `now` * make `urec.lastlogin` advance by `days * DAY_SECONDS` instead of reset to `now` * make `urec.numlogindays` increase by `days` instead of fixed 1 * main() * drop the assignment of now-unused global `now`
The current algorithm:
Demonstration Case 展示案例
The Login Count Increment Rate Issue 登入次數增加速率問題This issue resulted from the slightly flawed condition for excluding users from increment. The condition should exclude users whose online duration < 1 day. When For details, see:
A 1-hour tolerance of online duration was soon introduced but had soon been cancelled. Within 1 day, another commit changed this condition to However, (Cron jobs may delay for seconds, especially when the system load is high) For details, see:
In order to address this issue, the following commits redefined
The relevant logic partially reverted to commit 6871fbb ("Update using ulist."). However, the condition Since |
有使用者對 2014 年底後實施的登入次數累計方法有以下疑問:
https://www.ptt.cc/bbs/PttBug/M.1622376938.A.CC0.html
故把問題轉錄過來,也順便請教看看其他開發者對此有什麼看法?
The text was updated successfully, but these errors were encountered: