Add periodic connection re-sync #260
Conversation
WalkthroughThe changes refine the connection synchronization logic by updating the query in the Changes
Sequence Diagram(s)sequenceDiagram
participant T as Timer
participant CM as ConnectionManager
participant DB as Database
participant Sync as Sync Process
T->>CM: Trigger registerPollingCallback
CM->>CM: Compute thresholdDate using resyncConnectionIntervalMs
CM->>DB: Query connections (SYNC_NEEDED OR (SYNCED/ WARNINGS + outdated))
DB-->>CM: Return matching connections
CM->>Sync: Initiate sync process for each connection
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
brendan-kellam
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/backend/src/connectionManager.ts (2)
74-74: Consider handling extreme or invalid interval values.
While the schema enforces a minimum value of 1 forresyncConnectionIntervalMs, it may be safer to validate and clamp values in code to prevent unexpected behavior if the setting is ever overridden or corrupted.
82-95: Consider indexing and test coverage.
The multi-OR query onsyncStatusandsyncedAtcould benefit from indexing in the database for better performance on large datasets. Also ensure that you have an integration test verifying that connections older than the threshold indeed get re-synced.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/backend/src/connectionManager.ts(1 hunks)packages/backend/src/constants.ts(1 hunks)packages/schemas/src/v3/index.schema.ts(1 hunks)packages/schemas/src/v3/index.type.ts(1 hunks)schemas/v3/index.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (5)
packages/backend/src/constants.ts (1)
10-10: LGTM: New connection re-sync interval setting added.The addition of
resyncConnectionIntervalMswith a default value of 24 hours (in milliseconds) aligns with the PR objective to configure how frequently a connection should be re-synced.packages/schemas/src/v3/index.schema.ts (1)
26-30: Schema definition for re-sync interval looks good.The schema properly defines the new
resyncConnectionIntervalMsproperty with appropriate type, description, and minimum value constraint. This matches the implementation in the constants file.packages/schemas/src/v3/index.type.ts (1)
42-45: TypeScript interface update is correct.The TypeScript interface has been properly extended with the new optional property, including appropriate JSDoc comment that clearly describes its purpose.
schemas/v3/index.json (1)
26-30: JSON schema definition is consistent.The JSON schema definition for
resyncConnectionIntervalMsis consistent with the TypeScript definitions, having the same type, description, and minimum value constraint.packages/backend/src/connectionManager.ts (1)
77-81: Verify conflicting or duplicate statuses.
If a connection is already inIN_SYNC_QUEUEorSYNCING, this condition could redundantly place it back into the queue. Confirm that this doesn’t create duplicate or conflicting sync jobs.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/self-hosting/more/declarative-config.mdx (1)
58-62: Addition of JSON Schema Property:resyncConnectionIntervalMsThe new property is correctly incorporated into the
Settingsdefinition. Its type, description, and minimum value are consistent with similar schema properties. As a suggestion, if your configuration loader supports in-schema defaults, consider adding an explicit default (e.g."default": 86400000") to mirror the backend default of 24 hours.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CHANGELOG.md(1 hunks)docs/self-hosting/more/declarative-config.mdx(1 hunks)
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md
[duplication] ~14-~14: Possible typo: you repeated a word.
Context: .../pull/259)) ### Added - Added config setting `resyncConnectionInterva...
(ENGLISH_WORD_REPEAT_RULE)
🔇 Additional comments (1)
CHANGELOG.md (1)
14-16: Changelog Entry forresyncConnectionIntervalMsis Well-FormulatedThe new changelog entry clearly documents the addition of the
resyncConnectionIntervalMssetting along with its purpose and a link to PR [#260]. This helps maintain clarity in project history.🧰 Tools
🪛 LanguageTool
[duplication] ~14-~14: Possible typo: you repeated a word.
Context: .../pull/259)) ### Added - Added config setting `resyncConnectionInterva...(ENGLISH_WORD_REPEAT_RULE)
This PR adds the
resyncConnectionIntervalMsto control how often a given connection should be re-synced.Summary by CodeRabbit
New Features
resyncConnectionIntervalMs, which specifies the frequency at which connections should be re-synced, defaulting to 24 hours. This enhancement allows for more efficient management of connection synchronization.Documentation
resyncConnectionIntervalMsproperty in the JSON schema and other relevant sections.