Skip to content

זהות שורה יציבה בלי heRef, וגארד נגד patch דלתא גדול מדי - #28

Merged
Y-PLONI merged 4 commits into
Otzaria:otzariafrom
palmoni5:feat/stable-line-ids-and-patch-size-guard
Sep 10, 2026
Merged

זהות שורה יציבה בלי heRef, וגארד נגד patch דלתא גדול מדי#28
Y-PLONI merged 4 commits into
Otzaria:otzariafrom
palmoni5:feat/stable-line-ids-and-patch-size-guard

Conversation

@palmoni5

@palmoni5 palmoni5 commented Sep 7, 2026

Copy link
Copy Markdown
Member

מה

  1. זהות שורה בלי heRef — המפתח החוצה-בניות של שורת ספריא הופך מ-sha1("REF:"+heRef) ל-sha1("CT:"+rawSegment): התוכן הגולמי לפני קידומות הפסוק המוזרקות ((א) , תוויות דף), עם occurrenceIdx לכפילויות תוכן בתוך הספר. heRef נשאר עמודה מתעדכנת רגילה שזורמת ב-upsert_line.
  2. מעבר בלי renumbering — shim מבודד (LegacyLineKey.kt, LineOccurrenceCounter.kt, InMemoryIdAllocator.migrateLegacyLineKey): לכל שורה שהמפתח החדש שלה חסר מנוסה המפתח הישן מתוך snapshot בלתי־משתנה של ה-buildstate של הזרע, וה-id נרשם במפה החדשה רק אם טרם הוקצה לשורה אחרת. היבואן מחזיק שני מוני occurrence במהלך המעבר. סיכום המעבר נרשם (מפתחות בזרע / הוגרו / ids טריים), ומעל 1% החמצות נרשמת אזהרה. ה-shim מיועד למחיקה אחרי בנייה מלאה אחת מוצלחת.
  3. PatchSizeGuard — עוגן ש-patch שלו פרוס מעל DEFAULT_MAX_DELTA_UNCOMPRESSED_RATIO = 0.30 מגודל ה-DB החדש מדולג דרך מסלול .unpatchable הקיים (טוקן oversized-delta), לפני אימות ודחיסה. ב-manual-generate-release.yml: כשכל העוגנים דולגו בגלל הגארד, ה-release מתפרסם full-only עם ::warning:: במקום להיכשל; כשל מבני אמיתי עדיין מפיל. ה-workflows של האימות מקבלים -PmaxDeltaUncompressedRatio=1000 (בודקים מכניקה, לא כלכלה).
  4. תיעוד: DELTA_UPDATE_WORKFLOW.md, LINKER_DELTA_PLAN.md.

למה

Otzaria/otzaria#1211. נמדד על patch-v26-v27: upsert_line 1,558,787 ו-delete_line 1,556,251 עם אפס חפיפת מזהים, ב-5,128 ספרים; upsert_link/delete_link 5.27M כל אחת; סה"כ 2.4GB פרוסים לצעד גרסה יחיד (לעומת 7–43MB דחוסים ב-releases קודמים). הסיבה: 4e286b9 שינה heRef לכל שורה בסכמה הפשוטה, וכל שורה קיבלה id חדש עם קסקדה ל-link/line_toc/version_line. לקוחות החילו 3GB במשך שעה בלי חיווי.

הגארד הוא רשת ביטחון בלבד ללקוחות שאין להם דיאלוג בחירה. אפליקציית אוצריא (PR נלווה) מסמנת דלתא כבדה כבר ב-0.25 ומשאירה את הבחירה למשתמש, כי ברשת איטית 585MB עדיפים על 1.4GB.

תיקון בעקבות הביקורת (קומיט 5dda924) + ריבייס מול otzaria

הביקורת צדקה: ה-shim קרא מאותה מפה חיה שהבנייה כותבת לה, ולכן שורה A (עם heRef, תוכן X) שתפסה את CT:X#0 ושורה B (בלי heRef) שמפתח ה-legacy שלה הוא אותו CT:X#0 קיבלו את אותו id, ו-INSERT OR IGNORE היה בולע את השנייה בשקט.

  • המיגרציה קוראת רק מ-snapshot בלתי משתנה של ה-build_state הישן (seedLines).
  • פנקס id→מפתח לבנייה הנוכחית: מזהה שכבר הוקצה לשורה אחרת לא יוצא שוב. פגיעת legacy על מזהה תפוס נופלת ל-id חדש ונספרת ב-legacyLineKeyCollisions (מודפס בסיכום הבנייה).
  • הקצאה ישירה של אותו id לשני מפתחות (seed פגום) מפילה את הבנייה ב-IllegalStateException במקום להפיל שורה.
  • snapshotTo מוחק מפתחות legacy שנותרו לספרים שעובדו בבנייה, כך שהם לא מזריעים בנייה עתידית.
  • שתי בדיקות חדשות ב-LegacyLineKeyMigrationTest: התרחיש מהביקורת בדיוק (ids שונים, snapshot נקי, בנייה שלישית יציבה), ו-seed פגום שמפיל את הבנייה.
  • INSERT OR IGNORE ב-LineQueries.sq לא שונה בכוונה: הוא משרת גם מסלולים אחרים, והאינווריאנט נאכף עכשיו בנקודת ההקצאה, לפני שה-insert בכלל רואה כפילות.

הריבייס מול otzaria (09f60c3): produce_anchor עבר ב-upstream ל-.github/scripts/patch_fan_lib.sh, ולכן SKIP_DIR/record_skip ושלוש קריאות הרישום הועברו לשם; הבדיקה בחוזה קוראת מה-lib. אין שינוי בהתנהגות.

תיקונים נוספים בעקבות הביקורת (קומיטים 40b7005, 0cfa140)

  • שורה עם קידומת שנוצרה ביבוא וגם עברה cleanSefariaLine שמרה בטעות את הקידומת בתוך המפתח הטבעי; הוספת שורה לפניה הייתה משנה את ה-id. כעת מצב הניקוי ואורך הקידומת מקודדים יחד: מפתח השורה מסיר את הקידומת, אך עוגני תווים עדיין נדחים כלא־מדויקים. נוספה בדיקת רגרסיה דרך SefariaBookPayloadReader האמיתי עם <br> והזזת (א) ל-(ב).
  • גארד השרת הוקשח מ-0.5 ל-0.30. טווח v27 שנמדד (31.7%–40.3% מגודל ה-DB החדש) נדחה כעת; בדיקת הרגרסיה משתמשת ביחס האמיתי ולא בדוגמה מלאכותית של 75%. סימון הלקוח נשאר 0.25.

תאימות

אין שינוי בסכמת ה-DB, בפורמט ה-patch, במניפסט או בסכמת ה-buildstate (CURRENT_VERSION נשאר 1). לקוחות קיימים ממשיכים לעבוד מול הארטיפקטים החדשים.

לפני ה-release הראשון (חשוב)

  • להריץ delta-real-diff-test.yml עם baseline_ref = upstream/otzaria (בונה v1 בקוד הישן ו-v2 בחדש) ולוודא שמספר ה-Migrated … line ids ≈ מספר השורות נושאות-heRef, ושהאזהרה על מעבר לא-נקי לא נורית.
  • הבנייה הראשונה חייבת לרוץ על כל הספרים; בנייה חלקית תשאיר תערובת מפתחות (נכונה, אך המעבר לא יושלם).
  • מגבלה ידועה: שורה שה-heRef שלה משתנה באותה בנייה של המעבר תקבל id טרי (אין את ה-heRef הישן בשום מקום). המטריקה תחשוף זאת.

PRים נלווים

בדיקות

./gradlew build --no-daemon --no-configuration-cache עבר במלואו (כולל Android וכל מודולי Kotlin); בדיקות ה-JVM של generator-common, otzariasqlite ו-sefariasqlite עברו; ו-162 בדיקות Python של סקריפטי ה-CI עברו (24 דולגו כצפוי). נוספו רגרסיות לקידומת+ניקוי ולסף 30%. טרם הורצה בנייה מלאה של seforim.db; בדיקת delta-real-diff-test נשארת שער חובה לפני release.

@palmoni5

palmoni5 commented Sep 9, 2026

Copy link
Copy Markdown
Member Author

שורה תחתונה: לא — ה־PR אינו חסר סיכון, ובמצבו הנוכחי לא הייתי מאשר את PR #28. מצד אוצריא, דיווחי טעויות אינם נשברים ישירות; אבל יש באג ממשי במיגרציית מזהי השורות שעלול לייצר DB שגוי, ואז בעקיפין לפגוע גם בדיווחים, בקישורים ובתוכן.

דיווחי טעויות בספר

בדקתי את שני מסלולי הדיווח:

  • דיווח ישיר שולח book_title,‏ current_ref,‏ line_number, הטקסט שנבחר, ההקשר וגרסת הספרייה. אין בו line.id או hash של heRef. המודל וה־payload
  • המראה־מקום מחושב ממספר השורה ותוכן העניינים, לא ממזהה השורה. בניית הדיווח
  • הדיווח הטלפוני שולח book_id, מספר שורה וגרסת ספרייה — גם כאן לא line.id. PhoneReportData
  • גם דיווחים הממתינים לשליחה נשמרים עם אותם שדות, ולכן שינוי שיטת הקצאת line.id אינו מבטל אותם.

גם הנתונים האישיים העיקריים אינם תלויים ב־line.id:

  • סימניות נשמרות לפי ספר, אינדקס ומראה־מקום. Bookmark
  • הערות אישיות נשמרות לפי מזהה ספר ומספר שורה, עם טקסט עוגן. PersonalNote

לכן: בהנחה שה־DB שנוצר תקין, החלפת המפתח מ־heRef לתוכן לא אמורה לשבור דיווחים, סימניות או הערות אישיות. heRef עצמו גם נשאר בעמודת line.heRef.

אבל יש באג חוסם במחולל

במיגרציה הישנה והחדשה משתמשים באותה מפה mutable של lines. אפשר להגיע למצב שבו שתי שורות מקבלות אותו ID:

  1. שורה A עם heRef ותוכן X שמורה ב־buildstate הישן תחת REF:A → id1.
  2. שורה B ללא heRef, גם היא עם תוכן X, שמורה תחת CT:X#0 → id2.
  3. בעיבוד החדש A מבקשת CT:X#0, מוצאת אותו ומקבלת id2.
  4. B היא עכשיו המופע השני ומבקשת CT:X#1; המיגרציה מזיזה את המפתח הישן CT:X#0 ל־CT:X#1 — ומחזירה שוב id2.

הבעיה נמצאת בשילוב בין החיפוש הרגיל לבין remove/putIfAbsent במיגרציה: InMemoryIdAllocator.

וזה לא בהכרח יכשיל את הבנייה בקול: הכנסת השורות משתמשת ב־INSERT OR IGNORE, כך שהשורה השנייה עלולה פשוט להיעלם. LineQueries.sq

בצד אוצריא התוצאות האפשריות הן:

  • שורה חסרה או תוכן שגוי במספר שורה מסוים.
  • קישורים ומפרשים שמצביעים לשורה הלא נכונה.
  • line_toc,‏ version_line,‏ link_coverage או אינדקס החיפוש המשויכים ל־ID הלא נכון.
  • דיווח טעות שיישלח עם מספר השורה/הטקסט השגויים — לא מפני שמנגנון הדיווח נשבר, אלא מפני שה־DB שממנו הוא קורא כבר שגוי.

זה לבדו מצדיק Request changes.

התיקון הנכון הוא לשמור snapshot בלתי־משתנה של מפת המפתחות הישנה, ולבנות מפה חדשה ונפרדת; בנוסף צריך invariant שמוודא שכל ID מוקצה לכל היותר לשורה אחת בבנייה הנוכחית. כדאי גם להחליף INSERT OR IGNORE בבנייה ב־insert שמכשיל על התנגשות.

דיווח שגיאות טכני של העדכון

במסלול העדכון של אוצריא לא מצאתי בליעה חדשה של הכשל העיקרי:

  • כשל בבדיקה, הורדה או החלת patch נתפס עם stack trace ונכתב ל־errors.txt.
  • PatchDownloadCancelled אינו מדווח כשגיאה בכוונה.
  • סטיית תוכן אחרי commit נרשמת בנפרד.
  • כשל לאחר כמה צעדי דלתא שומר גם את הכשל המקורי וגם מידע על הצעדים שכבר הוחלו.

מסלול הטיפול בשגיאות, כתיבה ל־errors.txt.

שתי הסתייגויות:

  • שגיאות cleanup בתוך _deleteQuietly וניקוי patches ישנים נבלעות. זה לא מסתיר כשל apply, אבל יכול להשאיר קובצי ענק ולגרום אחר כך להודעת “אין מקום” בלי רישום הסיבה האמיתית.
  • החריגות המטופלות נכתבות ל־errors.txt, לא נשלחות ל־Sentry. זה היה המצב גם קודם, ולכן זו לא נסיגה של ה־PR, אבל אין כאן טלמטריה מלאה.

עוד סיכונים ממשיים

  • הגארד 0.5 לא עוצר את האירוע שבגללו נכתב ה־PR. ה־patches הבעייתיים שמדדתי היו בערך 31.7%–40.3% מגודל ה־DB החדש, ולכן כולם עוברים גארד של 50%. לקוחות ישנים עדיין עלולים לקבל patch של 2.4–3.1GB.
  • ה־planner מעדיף קודם מספר צעדים ורק אחר כך גודל דחוס. הוא יכול לבחור patch ישיר כבד במקום שרשרת של שני patches קלים בהרבה, ורק לאחר מכן להציג “דלתא כבדה”.
  • בדיקת המקום ל־WAL משתמשת בגודל ה־patch הפרוס כאומדן ללא מקדם ביטחון. SQLite עשוי לכתוב ל־WAL יותר מזה בגלל אינדקסים ושכתוב דפים. במקרה כזה העדכון אמור להתגלגל אחורה, אבל עדיין יכול להיכשל באמצע.
  • הוספת onVerifyProgress ל־method שניתן ל־override היא source-breaking עבור subclasses חיצוניים שלא עודכנו. StreamingPatchDownloader של אוצריא עודכן, לכן האפליקציה הידועה מכוסה; הטענה שכל שינויי ה־API “אופציונליים ולכן תואמים” אינה מדויקת.
  • כרגע dev של אוצריא מצביע על main של updater, שבו עדיין 0.4.0. אימתתי ש־flutter analyze נכשל עם 12 שגיאות קומפילציה. לאחר הצמדה ל־30bf3ff הוא נקי. לכן PR fix(sefariasqlite): long description in heDesc, real short desc in heShortDesc #11 חייב להתמזג לפני בנייה נוספת של אוצריא; זו גם הבעיה שתועדה ב־PR #1260.

אימות שעשיתי בצד אוצריא

על שילוב אוצריא עם updater בקומיט 30bf3ff:

לכן המסקנה המדויקת היא:

  • דיווחי הטעויות עצמם אינם תלויים בזהות line.id ולא אמורים להישבר.
  • לא ניתן להגדיר את השינוי כחסר סיכון.
  • PR זהות שורה יציבה בלי heRef, וגארד נגד patch דלתא גדול מדי #28 כולל כרגע באג correctness חוסם שיכול לייצר DB שגוי בשקט.
  • גם אחרי תיקונו דרושה בניית DB מלאה אמיתית ובדיקת end-to-end: יצירת baseline ישן, יצירת DB חדש, החלת patch באוצריא, בדיקת invariants, פתיחת ספרים/קישורים/חיפוש ושליחת דיווח משני סוגיו. בלי זה לא הייתי נותן אישור release.

מפתח השורה של ספריא היה sha1("REF:"+heRef), ולכן שינוי גורף ב-heRef (4e286b9, פסיק אחרי שם הספר) חידש את ה-id של 1.56M שורות ו-5.27M קישורים ב-5,128 ספרים, ו-patch v26→v27 תפח ל-2.4GB פרוסים (לעומת 7–43MB בעבר). המפתח עכשיו sha1("CT:"+rawSegment): התוכן הגולמי לפני קידומות הפסוק המוזרקות, עם occurrenceIdx לכפילויות תוכן בספר. heRef נשאר עמודה מתעדכנת רגילה.

מעבר בלי renumbering: shim מבודד (LegacyLineKey, LineOccurrenceCounter) מנסה לכל שורה את המפתח הישן ב-buildstate של הזרע, מעביר את ה-id למפתח החדש (remove + putIfAbsent) ומדווח מטריקת מעבר; מעל 1% החמצות נרשמת אזהרה. הבנייה הראשונה אחרי המיזוג חייבת לרוץ על כל הספרים, ומומלץ להריץ קודם delta-real-diff-test עם baseline upstream/otzaria.

PatchSizeGuard: עוגן ש-patch שלו פרוס מעל 0.5 מגודל ה-DB החדש מדולג דרך מסלול .unpatchable הקיים (טוקן oversized-delta); כשכל העוגנים מדולגים ה-release מתפרסם full-only עם אזהרה במקום להיכשל. ה-workflows של האימות מקבלים יחס 1000 כי הם בודקים מכניקה. הגארד הוא רשת ביטחון בלבד; אפליקציית אוצריא מסמנת דלתא כבדה כבר ב-0.25 ומשאירה את הבחירה למשתמש.

אין שינוי בסכמת ה-DB, בפורמט ה-patch, במניפסט או ב-buildstate — לקוחות קיימים ממשיכים לעבוד מול הארטיפקטים החדשים. רקע: Otzaria/otzaria#1211.
ה-shim קרא מאותה מפה חיה שהבנייה כותבת לה, ולכן שורה A (עם heRef) שתפסה את CT:X#0 ושורה B (בלי heRef) שמפתח ה-legacy שלה הוא אותו CT:X#0 קיבלו את אותו id, ו-INSERT OR IGNORE בלע את השנייה בשקט.

התיקון: המיגרציה קוראת רק מ-snapshot בלתי משתנה של ה-build_state הישן; פנקס id→מפתח לבנייה הנוכחית מוודא שמזהה שכבר הוקצה לשורה אחרת לא יוצא שוב (נופל ל-id חדש, נספר ב-legacyLineKeyCollisions); הקצאה ישירה של אותו id לשני מפתחות מפילה את הבנייה במקום להפיל שורה. ב-snapshotTo נמחקים מפתחות legacy שנותרו לספרים שעובדו.

שתי בדיקות חדשות: התרחיש מהביקורת, ו-seed פגום שמפיל את הבנייה.
@palmoni5
palmoni5 force-pushed the feat/stable-line-ids-and-patch-size-guard branch from 9e4611f to 5dda924 Compare September 10, 2026 00:39
@palmoni5

Copy link
Copy Markdown
Member Author

תודה, הביקורת צדקה ותוקן בקומיט 5dda924 (הענף גם עבר ריבייס מול otzaria).

התיקון: המיגרציה קוראת רק מ-snapshot בלתי משתנה של ה-build_state הישן, ופנקס id→מפתח לבנייה הנוכחית מוודא שמזהה שכבר הוקצה לשורה אחרת לא יוצא שוב. בתרחיש שתיארת, B מקבלת id חדש (נספר ב-legacyLineKeyCollisions), וה-snapshot הנכתב מכיל מפתח אחד לכל id. הקצאה ישירה של אותו id לשני מפתחות מפילה את הבנייה. שתי בדיקות חדשות: התרחיש שלך בדיוק, ו-seed פגום.

INSERT OR IGNORE הושאר: האינווריאנט נאכף בנקודת ההקצאה, לפני ה-insert.

נותר פתוח: גארד 0.5 לא היה תופס את v27 (32%–40%). ההצעה: 0.3 בשרת ו-0.15 בלקוח, או להשאיר. וכן בדיקת end-to-end על DB אמיתי (delta-real-diff-test עם baseline otzaria) לפני release, כפי שכתוב בגוף ה-PR.

@palmoni5
palmoni5 marked this pull request as ready for review September 10, 2026 00:41
@palmoni5

Copy link
Copy Markdown
Member Author

התלבטות: להשאיר את ה-shim או להסיר אותו

ה-LegacyLineKey הוא גשר של בנייה אחת: ה-build_state של הרלסים עד v27 שומר את מזהי השורות תחת המפתח הישן (עם heRef), וה-shim מחפש כל שורה גם שם ומעביר את ה-id למפתח החדש. כך v28 יוצא עם אותם מזהי שורות כמו v27. הבאג שנמצא בביקורת היה בגשר הזה, ותוקן. השאלה היא אם הגשר בכלל שווה את המורכבות שלו.

בלי ה-shim, בבנייה הראשונה כל שורה של ספריא מקבלת id חדש. זה בדיוק מה שקרה ב-v27 בגלל הפסיק ב-heRef, רק הפעם במכוון ופעם אחת.

עם shim בלי shim
מסד ואפליקציה תואם תואם, הסכמה לא משתנה
patch דלתא ל-v28 קטן כרגיל ענק, כמו v27 (2.5–3GB פרוסים)
מה מתפרסם patches + מסד מלא ה-PatchSizeGuard זורק את כל ה-patches, רלס מלא בלבד
לקוח ישן (לפני otzaria#1223) דלתא הורדה מלאה של ~1.4GB. אין תקיעה כמו ב-#1211 כי אין patch לבחור
לקוח חדש דלתא הורדה מלאה, בלי שאלת מסלול
סימניות, הערות, דיווחים לא מושפעים לא מושפעים (לפי ספר ומספר שורה, לא line.id)

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

להסיר: PR פשוט בהרבה, בלי מסלול מיגרציה בכלל. המחיר: v28 יהיה הורדה מלאה של 1.4GB לכל המשתמשים, כולל ברשת איטית. אחרי v28 המזהים יציבים והדלתאות חוזרות להיות קטנות.

הנטייה שלי: להסיר, כי מחיר חד-פעמי ידוע עדיף על קוד מעבר שנשאר. אבל זו החלטת מוצר, לא טכנית, ולכן לא שיניתי כלום עד להחלטה.

@Y-PLONI

Y-PLONI commented Sep 10, 2026

Copy link
Copy Markdown
Member

אימות end-to-end מלא עבר על head 0cfa140 מול baseline 09f60c3, עם הקלטים המקובעים של ריצת production 34024655297.

  • baseline v1: כל generateSeforimDb עבר, כולל seedGenerations ויתר שלבי metadata/indexing (47m10s)
  • PR v2: כל generateSeforimDb עבר עם אותו תוכן ואותו build_state seed (47m24s)
  • producePatchAndVerify עבר: החלת ה-patch החזירה בדיוק את hash היעד e3a1b2e810d1ea5babdfca1c336713b1666397dfa288008e43f372b8f8210fa6
  • השינוי היחיד בין ה-DBs היה schema_meta של db_version 1→2: upserts=1, deletes=0
  • patch.db: 503,808 bytes; דחוס zstd: 4,021 bytes
  • בדיקות ה-PR הרגילות ירוקות והענף MERGEABLE/CLEAN

שתי ריצות האימות המרוחקות המיותרות (production runner, server-2) בוטלו לאחר שהאימות המקומי המלא נתן תוצאה מכרעת.

@Y-PLONI
Y-PLONI merged commit 5afcbac into Otzaria:otzaria Sep 10, 2026
3 of 4 checks passed
@palmoni5
palmoni5 deleted the feat/stable-line-ids-and-patch-size-guard branch September 10, 2026 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants