Skip to content

refactor: batch fsync performance, update ID length, and fix transactional recovery - #350

Merged
AnkanSaha merged 2 commits into
mainfrom
maintainer/ankan
Jul 25, 2026
Merged

refactor: batch fsync performance, update ID length, and fix transactional recovery#350
AnkanSaha merged 2 commits into
mainfrom
maintainer/ankan

Conversation

@AnkanSaha

Copy link
Copy Markdown
Member

This pull request upgrades the auto-generated document ID system to use 30-character uppercase alphanumeric strings (instead of 15-character letters-only), improving uniqueness and reducing collision risks. It also includes a new changelog entry, version bump to 14.1.4, and a test to verify the new ID format.

Document ID generation improvements:

  • Document IDs are now generated as 30-character uppercase alphanumeric strings, increasing uniqueness and lowering collision odds. This change updates all relevant locations to use the new length and character set, controlled by General.DocumentId_Length. (source/Helper/UniqueGenerator.helper.ts [1] source/config/Keys/Keys.ts [2] source/Services/CRUD Operation/Create.operation.ts [3] source/Services/Transaction/Transaction.service.ts [4] [5]

  • Added a test to ensure generated document IDs are 30 characters long and contain only uppercase letters and digits. (Test/modules/crud.test.js Test/modules/crud.test.jsR60-R67)

Changelog and versioning:

  • Added a new changelog entry for version 14.1.4, summarizing the document ID changes and several transaction durability and WAL batching improvements. (Document/src/data/changelog.ts Document/src/data/changelog.tsR14-R26)

  • Updated the package version to 14.1.4 to reflect these changes. (package.json package.jsonL3-R3)

@AnkanSaha AnkanSaha self-assigned this Jul 25, 2026
@AnkanSaha
AnkanSaha enabled auto-merge July 25, 2026 10:58
@github-actions

Copy link
Copy Markdown

🤖 Review Buddy - General Code Review

👥 Attention: @AnkanSaha

Oye @AnkanSaha, kaam toh tune kaafi 'Zakaas' kiya hai, especially wo performance optimization! 1000 fsyncs ko batch karke tune database ko rocket bana diya hai (~4.5ms se ~0.7ms, kya baat hai!). Transaction recovery aur 30-char alphanumeric IDs wala logic bhi ekdum solid hai. Lekin bhai, thodi jagah tune wahi purani 'rookie' mistakes kar di hain. Versioning mein locha, loop ke andar memory waste, aur naming conventions ka kachra—ye sab thoda saaf kar de toh maza aa jaye. Overall, heavy lifting tune kar li hai, bas ye finishing touches de de!


Generated by Review Buddy | Tone: roast | Language: hinglish

@github-actions github-actions Bot changed the title Maintainer/ankan refactor: batch fsync performance, update ID length, and fix transactional recovery Jul 25, 2026
@github-actions

Copy link
Copy Markdown

⚡ Review Buddy - Performance Analysis

👥 Attention: @AnkanSaha

Bhai, performance ke maamle mein tune ek taraf aag lagayi hai aur dusri taraf pani pher diya hai:

  • The Good: Document/src/data/changelog.ts mein 1000 fsyncs ko batch karke tune performance ko maut ke kuan se nikaal kar Formula 1 track pe khada kar diya hai. 0.7ms is insane!
  • The Bad: source/Services/CRUD Operation/Create.operation.ts mein do-while loop ke andar new FileManager()? Kyun bhai? Memory tere baap ki factory mein banti hai? Har iteration pe naya object instantiate karke garbage collector ka kaam kyun badha raha hai? Isse loop ke bahar nikaal le, performance thodi aur chamkegi.

Generated by Review Buddy | Tone: roast | Language: hinglish

@github-actions

Copy link
Copy Markdown

🔐 Review Buddy - Security Audit

👥 Attention: @AnkanSaha

Security ke maamle mein tune dil jeet liya hai:

  • source/Helper/UniqueGenerator.helper.ts mein crypto.randomInt use kiya hai, jo ki Math.random() se lakh guna behtar hai. Cryptographically secure code likhne ke liye full marks.
  • Atomic writes ka use (temp file + rename) aur path sanitization Create.operation.ts mein dikhata hai ki tu race conditions aur injection attacks ko seriously le raha hai. Shabash!

Generated by Review Buddy | Tone: roast | Language: hinglish

@github-actions

Copy link
Copy Markdown

📊 Review Buddy - Code Quality & Maintainability Analysis

👥 Attention: @AnkanSaha

🎯 Overall Benchmark: 82/100 (Good)

Code quality overall badiya hai, par ye chhoti-chhoti galtiyan khatakti hain:

  • Naming Conventions: UniqueGenerator.helper.ts mein RandomWord ka 'R' capital hai. Bhai, TypeScript mein methods camelCase hote hain. C# se breakup kar le ab, ye TS ki duniya hai.
  • Error Handling: Create.operation.ts mein catch block poora error object client ko bhej raha hai. Client ko error ka 'bio-data' nahi chahiye, sirf error.message kaafi hai. Poora stack trace bahar fekna unprofessional hai.

Generated by Review Buddy | Tone: roast | Language: hinglish

@github-actions

Copy link
Copy Markdown

💡 Review Buddy - Best Practices & Alternative Suggestions

👥 Attention: @AnkanSaha

Bhai, best practices follow karne mein thodi aur mehnat chahiye:

  • Semantic Versioning (SemVer): package.json mein major version 13 se 14 kiya, toh minor/patch ko zero karna tha na?

    • Before: "version": "14.1.4"
    • After: "version": "14.0.0"
  • Method Naming: PascalCase methods class ke bahar ache nahi lagte.

    • Before: public RandomWord(...)
    • After: public randomWord(...)
  • Constants: Hardcoded numbers ko constants se replace karna badiya move hai.

    • Example: 15 ki jagah General.DocumentId_Length use karke tune code ko maintainable banaya hai.

Generated by Review Buddy | Tone: roast | Language: hinglish

@github-actions github-actions Bot added the enhancement New feature or request label Jul 25, 2026
@github-actions

Copy link
Copy Markdown

⚠️ Review Buddy - Final Recommendation

👥 Attention: @AnkanSaha

Recommendation: REQUEST CHANGES

Changes chahiye, bhai! Abhi approve nahi kar sakte.

Reasoning:

  • Semantic Versioning error in package.json (Major version jump should reset minor/patch).
  • Inefficient object instantiation of FileManager inside a do-while loop in Create.operation.ts.
  • Naming convention violation (PascalCase method name in TypeScript).
  • Catch block exposing raw error objects instead of specific messages.

📋 Review Checklist for Reviewers:

  • Code changes align with the PR description
  • No security vulnerabilities introduced
  • Performance considerations addressed
  • Code follows project conventions
  • Tests are adequate (if applicable)
  • Documentation updated (if needed)

🎯 Next Steps:

⚠️ Pehle suggestions address karo, phir approve karna.

Generated by Review Buddy | Tone: roast | Language: hinglish

@AnkanSaha
AnkanSaha merged commit f0c7db7 into main Jul 25, 2026
14 checks passed
@AnkanSaha
AnkanSaha deleted the maintainer/ankan branch July 25, 2026 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant