kolzchut / mediawiki-extensions-KZBrokenLinks

GNU General Public License v2.0
0 stars 0 forks source link

תיקונים להרחבה לבדיקת קישורים שבורים #1

Closed drorsnir closed 8 months ago

drorsnir commented 11 months ago

מוקצה לי לבדיקת התיקונים.

drorsnir commented 10 months ago

@biwashingtonial, כמה בעיות שאני מחזיר אליך:

  1. אני עדיין נתקל בבעיית ה-ratelimting. המימוש הנוכחי נאיבי ורק מנסה להפחית את הסיכוי שזה יקרה; גוגל ממליצים לתפוס את ה-exception ולהטמיע אלגוריתם truncated exponential backoff - למיטב הבנתי \Google\Task\Runner הוא יישום של זה, אבל לא בדקתי את השימוש בו (ואני גם לא רואה שהוא מחזיר אינדיקציה למשתמש לגבי העיכוב). אפשר לכל הפחות לתפוס את ה-exception, ולהעתיק את המימוש של האלגוריתם משם.
  2. ההתעלמות מפרוטוקולים כמו tel או mailto לא עובדת, כי את.ה בודק.ת כך:
    // Break out the URL protocol.
    $urlexp = explode( '://', $url, 2 );

    וב-mailto ו-tel אין '//'. אם parse_url() לא מספיק טובה, אפשר להשתמש בחבילה נפרדת כמו league\uri.

biwashingtonial commented 10 months ago

מוכן לבדיקה @drorsnir

drorsnir commented 10 months ago

אם אני משתמש ב-chunksize=500 (ברירת המחדל), נראה שמנקודה מסוימת זה פשוט נתקע:

Loading externallinks data from el_id 375790 Loading externallinks data from el_id 375790... Pausing 2 sec. to respect Google API rate limit of 60 callouts per minute. Loading externallinks data from el_id 375790...

כמו כן, נראה שזה עדיין משתמש במנגנון שלך ל-ratelimiting, ולא ב-exponential backoff retries.

בנוסף, נתקלתי בכמה בעיות של מצב inconsistent ב-spreadsheet:

  1. ראה את ארבע השורות כאן: [image: image.png]
  2. ואיכשהו כתוצאה מזה (אולי), אז כאן כמה קישורים נדחסו לשורה הראשונה: [image: image.png]
  3. והתוצאה היא שגיאת 400 עם התשובה ""Unable to parse range: LINKS_STATUS!E "

ניסיתי למחוק את הגיליון כמה פעמים (חוץ מהנוסחאות), ואפילו לשכפל שוב את הגיליון המקורי שנתת לנו ולהתחיל ממנו, אבל זה לא עזר לי. הצעות?

biwashingtonial commented 10 months ago

מעניין האם נתקע כל פעם באותה הנקודה? ולכמה זמן אין עידכון? בגדול אם רואים פאוזה ארוכה בפלט למסך זאת אומרת שה-exponential backoff כן מופעל. זה מבוצע בתוך קליינט גוגל באופן שקט בניגוד ל-rate limiting שמספק עדכונין למסך על מה קורה. אגב השארתי את מנגנון ה-rate limiting למען ביצוע מהיר יותר בסה''כ. הכוונה היא שמנגנון אחד מביא את הקצב עד סף המגבלה והשני מטפל במקרה שאיכשהו חורגים. @drorsnir

drorsnir commented 10 months ago
  1. לא, אין הפסקה ארוכה בפלט למסך. ב"נתקע" התכוונתי שהוא כותב שוב ושוב Loading externallinks data from el_id 375790, כלומר מאותה נקודה, ומדי פעם מוסיף הודעה על הגבלת קצב.
  2. לגבי מנגנון ה-ratelimiting - אוקיי.
  3. הבעיה האמיתית היא ה-inconsistency בגיליון הנתונים - למעשה לא הצלחתי עד עכשיו להריץ את בדיקת הקישורים עצמה.

On Tue, 23 Jan 2024 at 15:32, Joel Rothschild @.***> wrote:

מעניין האם נתקע כל פעם באותה הנקודה? ולכמה זמן אין עידכון? בגדול אם רואים פאוזה ארוכה בפלט למסך זאת אומרת שה-exponential backoff כן מופעל. זה מבוצע בתוך קליינט גוגל באופן שקט בניגוד ל-rate limiting שמספק עדכונין למסך על מה קורה. אגב השארתי את מנגנון ה-rate limiting למען ביצוע מהיר יותר בסה''כ. הכוונה היא שמנגנון אחד מביא את הקצב עד סף המגבלה והשני מטפל במקרה שאיכשהו חורגים. @drorsnir https://github.com/drorsnir

— Reply to this email directly, view it on GitHub https://github.com/kolzchut/mediawiki-extensions-KZBrokenLinks/issues/1#issuecomment-1906065281, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA2CAK56Z55Z7KEVVRFIUWLYP63W3AVCNFSM6AAAAAA7UFVMJOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBWGA3DKMRYGE . You are receiving this because you were mentioned.Message ID: @.*** com>

biwashingtonial commented 9 months ago

דחפתי לגיטהאב תיקון באג לבעיית הלופ. גם תיקנתי את הנוסחאות בטבלאות שלך, פרטים במייל. @drorsnir

drorsnir commented 9 months ago

@biwashingtonial, עובד עכשיו, אבל יש לנו בעיה רצינית. יש בבדיקה שילוב קטלני בין שלושה גורמים:

  1. קריאת השורות אחת-אחת מ"מסד הנתונים" (הגליון) שמוגבלת ל-60 בשניה
  2. כל דומיין נבדק רק פעם אחת בריצה (במקום דחיה של X שניות)
  3. יש לנו המון קישורים מאותם דומיינים - אלפי קישורים ל-btl.gov.il, אלפי קישורים ל-gov.il, אלפי קישורים ל-nevo.co.il...
התוצאה היא שכל ריצה בודקת פחות ופחות קישורים (או ליתר דיוק, המספר יורד חדות אחרי כמה ריצות), כי המון שורות מסומנות כ-skipped ואז בהרצה הבאה נקראות שוב ומדלגים עליהן שוב, ויש המון המתנות ל-rate limiting. ראה.י את הטבלה הבאה שמתעדת את הריצות היום: Run # # of links (running total) # of links (this run) run time (min) - total Avg per minute
1 75 75 5 15
2 122 47 10 12.2
3 162 40 15 10.8
4 186 24 20 9.3
5 206 20 25 8.24
6 223 17 30 7.433333333
7 239 16 35 6.828571429
8 260 21 40 6.5
9 277 17 45 6.155555556
10 288 11 50 5.76
11 300 12 55 5.454545455
12 314 14 60 5.233333333
13 325 11 65 5
14 337 12 70 4.814285714
15 410 73 80 5.125

בריצה מס' 13 הממוצע הכולל הוא כבר רק 5 קישורים בדקה, כשלאורך זמן הוא כנראה יירד ל-2.4 קישורים בדקה. יש כ-18,000 קישורים, 5 קישורים בדקה בממוצע = 3600 דקות = 60 שעות = 2.5 ימים. וזה במקרה הטוב, ובהנחה שאין שיבושים אחרי אלפי ריצות.

אני חושש שזה לא יהיה ממש סביר.

הפתרון החלקי לבעיה הזאת הוא הוא זמן ריצה ארוך יותר, אבל גם הוא לא באמת יעזור אחרי הריצה הראשונה, כנראה. ניסיתי להריץ פעמיים עם זמן ריצה של 10 דקות, כדי לראות:

  1. בריצה הראשונה (שורה מס' 15 בטבלה) קיבלתי שיפור, 73 קישורים.
  2. בריצה השניה קיבלתי כבר הודעת שגיאה - שוב משהו יצא מסנכרון במסמך עצמו (ראה הודעת שגיאה בתחתית ההודעה). השתמשתי במסמך הזה: KZBrokenLinks test - Dror + Joel
php extensions/KZBrokenLinks/maintenance/HealthCheckLinks.php --runtime 600
PHP Notice:  Undefined offset: 1 in /var/www/kzlocal/w/extensions/KZBrokenLinks/maintenance/HealthCheckLinks.php on line 86
PHP Notice:  Undefined offset: 2 in /var/www/kzlocal/w/extensions/KZBrokenLinks/maintenance/HealthCheckLinks.php on line 87
Google\Service\Exception from line 134 of /var/www/kzlocal/w/vendor/google/apiclient/src/Http/REST.php: {
  "error": {
    "code": 400,
    "message": "Unable to parse range: LINKS_STATUS!E2 3",
    "errors": [
      {
        "message": "Unable to parse range: LINKS_STATUS!E2 3",
        "domain": "global",
        "reason": "badRequest"
      }
    ],
    "status": "INVALID_ARGUMENT"
  }
}

#0 /var/www/kzlocal/w/vendor/google/apiclient/src/Http/REST.php(107): Google\Http\REST::decodeHttpResponse()
#1 [internal function]: Google\Http\REST::doExecute()
#2 /var/www/kzlocal/w/vendor/google/apiclient/src/Task/Runner.php(187): call_user_func_array()
#3 /var/www/kzlocal/w/vendor/google/apiclient/src/Http/REST.php(66): Google\Task\Runner->run()
#4 /var/www/kzlocal/w/vendor/google/apiclient/src/Client.php(922): Google\Http\REST::execute()
#5 /var/www/kzlocal/w/vendor/google/apiclient/src/Service/Resource.php(238): Google\Client->execute()
#6 /var/www/kzlocal/w/vendor/google/apiclient-services/src/Sheets/Resource/SpreadsheetsValues.php(271): Google\Service\Resource->call()
#7 /var/www/kzlocal/w/extensions/KZBrokenLinks/maintenance/HealthCheckLinks.php(127): Google\Service\Sheets\Resource\SpreadsheetsValues->update()
#8 /var/www/kzlocal/w/maintenance/doMaintenance.php(107): HealthCheckLinks->execute()
#9 /var/www/kzlocal/w/extensions/KZBrokenLinks/maintenance/HealthCheckLinks.php(206): require_once('/var/www/kzloca...')
#10 {main}
biwashingtonial commented 9 months ago

האמת שלא התכוונתי שיהו ריצות ארוכות בכלל. כמו שדיברנו מקודם (לפני כבר חודשיים שלושה) סקריפט HealthCheckLinks מהונדס לרוץ רק לכמה דקות ואז לרוץ שוב ושוב, בסופו של דבר דרך לו''ז cron על כל כמה דקות. לא בדקתי ריצות ארוכות ולא מפתיע שזה מדרדר לו את רמת הביצוע. אפשר בדיקות בריצות קצרות ברצף, דקה בריצה, שתי דקות, שלוש? מעניין אם זה יפתור את הבעיות.

‫בתאריך יום ג׳, 30 בינו׳ 2024 ב-16:04 מאת ‪Dror‬‏ @.*** ‬‏>:‬

Assigned #1 https://github.com/kolzchut/mediawiki-extensions-KZBrokenLinks/issues/1 to @biwashingtonial https://github.com/biwashingtonial.

— Reply to this email directly, view it on GitHub https://github.com/kolzchut/mediawiki-extensions-KZBrokenLinks/issues/1#event-11642273049, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIDJXCMND6N6VGL4GLTRKLYRD4VVAVCNFSM6AAAAAA7UFVMJOVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJRGY2DEMRXGMYDIOI . You are receiving this because you were assigned.Message ID: <kolzchut/mediawiki-extensions-KZBrokenLinks/issue/1/issue_event/11642273049 @github.com>

biwashingtonial commented 9 months ago

דחפתי לריפו ריפקטור די גדול לטפל בענייני הביצוע. השיפור בביצוע ענקי בבדיקות שלי.

גם טיפלתי בבאג ערמומי שהיה בשתיים מהשאילתות בטבלה וגרם לעיתים לדחיסת שורות רבות לתוך שורה אחת ב-NEXT_CHECK.

מוכן להמשך הבדיקות שלך.

‫בתאריך יום ד׳, 31 בינו׳ 2024 ב-13:34 מאת ‪Joel Rothschild‬‏ <‪ @.***‬‏>:‬

האמת שלא התכוונתי שיהו ריצות ארוכות בכלל. כמו שדיברנו מקודם (לפני כבר חודשיים שלושה) סקריפט HealthCheckLinks מהונדס לרוץ רק לכמה דקות ואז לרוץ שוב ושוב, בסופו של דבר דרך לו''ז cron על כל כמה דקות. לא בדקתי ריצות ארוכות ולא מפתיע שזה מדרדר לו את רמת הביצוע. אפשר בדיקות בריצות קצרות ברצף, דקה בריצה, שתי דקות, שלוש? מעניין אם זה יפתור את הבעיות.

‫בתאריך יום ג׳, 30 בינו׳ 2024 ב-16:04 מאת ‪Dror‬‏ <‪ @.***‬‏>:‬

Assigned #1 https://github.com/kolzchut/mediawiki-extensions-KZBrokenLinks/issues/1 to @biwashingtonial https://github.com/biwashingtonial.

— Reply to this email directly, view it on GitHub https://github.com/kolzchut/mediawiki-extensions-KZBrokenLinks/issues/1#event-11642273049, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIDJXCMND6N6VGL4GLTRKLYRD4VVAVCNFSM6AAAAAA7UFVMJOVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJRGY2DEMRXGMYDIOI . You are receiving this because you were assigned.Message ID: <kolzchut/mediawiki-extensions-KZBrokenLinks/issue/1/issue_event/11642273049 @github.com>

drorsnir commented 9 months ago

@biwashingtonial, that's an amazing improvement!

Now I just need to make sure the results actually make sense, and that it's possible to work with the spreadsheet to correct links. Thanks!

Run # # of links (running total) # of links (this run) Run time (this run) run time (min) - total Avg per minute (this run) Avg per minute (total)
1 80 80 1 1 80 80
2 200 120 1 2 120 100
3 280 80 1 3 80 93.33333333
4 380 100 1 4 100 95
5 440 60 1 5 60 88
6 500 60 1 6 60 83.33333333
7 560 60 1 7 60 80
8..12 980 420 5 12 84 81.66666667
13..22 1880 900 10 22 90 85.45454545
23..32 2540 660 15 37 44 68.64864865
33..62 5258 2718 45 82 60.4 64.12195122

Interesting to note that while the default runtime is now 60 seconds, it actually runs for a bit more (up to 95 seconds, in my tests so far). It was more apparent when I ran the script in 10x & 30x loops. But that's fine, really.

biwashingtonial commented 9 months ago

מעולה! הדבר שאגיד להיזהר ממנו בשלב של תיקונים לקישורים זה שלא תמיין מחדש את הטבלה LINKS_STATUS בעוד שרץ סקריפט הסנכרון. סביר שזה יוביל לכאוס. הכוונה שלי בעיצוב הטבלה הייתה שנריץ את הסקריפט בשעות הלילות ולאחר שהוא רץ נוכל למיין את הטבלה בלי להפריע לריצה הבאה של הסקריפט. רק לא במקביל.

biwashingtonial commented 9 months ago

אזהרה טובה - אם כי אף פעם לא עשיתי מיון, רק פילטר (שהוא של המשתמש שלי, ולא גלובלי במסמך).

biwashingtonial commented 9 months ago

תכל'ס פשוט צריך שמספרי השורות בטבלה לא ישתנו תוך כדי ריצת הסקריפט

drorsnir commented 9 months ago

@biwashingtonial, אני רק מוודא שהנוסחאות לא השתנו במסגרת השינויים האלה - אחרת צריך לעדכן את התבנית. האם היה שינוי כלשהו?

drorsnir commented 9 months ago

וגם - צריך לתעד את הפרמטרים האפשריים החדשים שיש בסקריפטים. לא שראיתי סיבה להריץ משהו שונה מברירת המחדל, אבל שיהיה.

biwashingtonial commented 9 months ago

טופל. גם עדכנתי את תבנית הטבלאות.

‫בתאריך יום ד׳, 7 בפבר׳ 2024 ב-13:01 מאת ‪Dror‬‏ @.*** ‬‏>:‬

וגם - צריך לתעד את הפרמטרים האפשריים החדשים שיש בסקריפטים. לא שראיתי סיבה להריץ משהו שונה מברירת המחדל, אבל שיהיה.

— Reply to this email directly, view it on GitHub https://github.com/kolzchut/mediawiki-extensions-KZBrokenLinks/issues/1#issuecomment-1931799707, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIDJXDHJ6YTCKSQQKCCU2TYSNNHPAVCNFSM6AAAAAA7UFVMJOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMZRG44TSNZQG4 . You are receiving this because you were mentioned.Message ID: @.*** com>