Replace debounce with throttle for cloud sync to ensure regular uploads #197
Conversation
原本的 debounce 機制會在每次觸發時重置計時器，導致持續播放時 永遠無法達到 30 秒的等待時間，書籤只有在停止播放後才會上傳。 現改用 throttle 機制： - 播放期間至少每 30 秒上傳一次 - 設置尾隨計時器確保停止播放後最終狀態也會上傳 - 維持頁面隱藏時的緊急上傳功能
Summary of ChangesHello @GJRobert, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the cloud synchronization logic from a debounce pattern to a more robust throttle implementation. The primary goal is to guarantee consistent data uploads at regular intervals during continuous activity, such as playback, while also ensuring that the final state is synchronized once activity ceases. This change aims to improve data integrity and user experience by providing more predictable sync behavior without excessively increasing API call frequency. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Pull Request Review: Replace debounce with throttle for cloud syncOverviewThis PR changes the cloud synchronization mechanism from debounce to throttle, adding a dual-timer system to ensure more reliable uploads during playback. The implementation is generally solid, but there are several areas that need attention. Code Quality & Best Practices✅ Strengths
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
這個 PR 成功地將雲端同步機制從 debounce 改為更適合的 throttle 方法。新的實作採用了節流（throttle）和尾隨（trailing）計時器的雙計時器系統，這個設計相當周全，既能確保在使用者操作期間能定期上傳資料，也能捕捉到最終狀態，從而提升了資料的可靠性。
我的審查意見包含一個重構 triggerCloudSync 函式的建議，以減少重複的程式碼，進而提升可維護性。
另外，有一個重要的潛在風險需要考慮：競爭條件（race condition）。新的節流邏輯可能會在先前的 syncToCloud 呼叫尚未完成時，再次觸發它。由於 syncToCloud 函式本身似乎沒有像 syncFromCloud 那樣的並行鎖（concurrency lock），這可能導致多個上傳請求同時進行。我建議在 syncToCloud 中加入鎖定機制（例如使用像 isSyncing 這樣的旗標），以確保一次只有一個上傳在執行，讓同步過程更加穩健。
| function triggerCloudSync() { | ||
| if (!cloudSyncState.isLoggedIn) return; | ||
|
|
||
| hasPendingSync = true; | ||
| const now = Date.now(); | ||
|
|
||
| if (syncDebounceTimer) { | ||
| clearTimeout(syncDebounceTimer); | ||
| // 清除尾隨計時器（因為有新的變更進來） | ||
| if (syncTrailingTimer) { | ||
| clearTimeout(syncTrailingTimer); | ||
| syncTrailingTimer = null; | ||
| } | ||
|
|
||
| syncDebounceTimer = setTimeout(() => { | ||
| // 如果距離上次同步已超過 30 秒，立即同步 | ||
| if (now - lastSyncTime >= SYNC_INTERVAL) { | ||
| // 清除節流計時器（如果有的話） | ||
| if (syncThrottleTimer) { | ||
| clearTimeout(syncThrottleTimer); | ||
| syncThrottleTimer = null; | ||
| } | ||
|
|
||
| lastSyncTime = now; | ||
| syncToCloud(); | ||
| hasPendingSync = false; | ||
| }, 30000); // 30 秒後同步，減少 Supabase API 呼叫次數 | ||
| } else if (!syncThrottleTimer) { | ||
| // 還沒到 30 秒，且沒有計時器在跑，設置計時器 | ||
| const remainingTime = SYNC_INTERVAL - (now - lastSyncTime); | ||
| syncThrottleTimer = setTimeout(() => { | ||
| syncThrottleTimer = null; | ||
| lastSyncTime = Date.now(); | ||
| syncToCloud(); | ||
| hasPendingSync = false; | ||
| }, remainingTime); | ||
| } | ||
|
|
||
| // 設置尾隨計時器：確保最後一次變更後也會上傳 | ||
| // 這樣停止播放後的最終狀態也能被保存 | ||
| syncTrailingTimer = setTimeout(() => { | ||
| syncTrailingTimer = null; | ||
| if (hasPendingSync) { | ||
| lastSyncTime = Date.now(); | ||
| syncToCloud(); | ||
| hasPendingSync = false; | ||
| } | ||
| }, SYNC_INTERVAL); | ||
| } |
There was a problem hiding this comment.
這段程式碼實現了帶有前緣和後緣觸發的節流邏輯，功能看起來是正確的。不過，在 if 判斷式、syncThrottleTimer 和 syncTrailingTimer 的回呼函式中，都有重複的同步邏輯（更新 lastSyncTime、呼叫 syncToCloud()、重設 hasPendingSync）。同樣的重複邏輯也出現在 setupPageUnloadSync 函式中。
為了提高程式碼的可讀性和可維護性，並遵循 DRY (Don't Repeat Yourself) 原則，建議將這段重複的邏輯提取到一個輔助函式中。這樣可以讓 triggerCloudSync 的主要邏輯更清晰，也更容易在未來統一修改同步行為。
我建議的重構如下：
function triggerCloudSync() {
if (!cloudSyncState.isLoggedIn) return;
const executeSync = (syncTime) => {
lastSyncTime = syncTime;
syncToCloud();
hasPendingSync = false;
};
hasPendingSync = true;
const now = Date.now();
// 清除尾隨計時器（因為有新的變更進來）
if (syncTrailingTimer) {
clearTimeout(syncTrailingTimer);
syncTrailingTimer = null;
}
// 如果距離上次同步已超過 30 秒，立即同步
if (now - lastSyncTime >= SYNC_INTERVAL) {
// 清除節流計時器（如果有的話）
if (syncThrottleTimer) {
clearTimeout(syncThrottleTimer);
syncThrottleTimer = null;
}
executeSync(now);
} else if (!syncThrottleTimer) {
// 還沒到 30 秒，且沒有計時器在跑，設置計時器
const remainingTime = SYNC_INTERVAL - (now - lastSyncTime);
syncThrottleTimer = setTimeout(() => {
syncThrottleTimer = null;
executeSync(Date.now());
}, remainingTime);
}
// 設置尾隨計時器：確保最後一次變更後也會上傳
// 這樣停止播放後的最終狀態也能被保存
syncTrailingTimer = setTimeout(() => {
syncTrailingTimer = null;
if (hasPendingSync) {
executeSync(Date.now());
}
}, SYNC_INTERVAL);
}
Summary
Changed the cloud synchronization mechanism from debounce to throttle to ensure more reliable and consistent uploads during playback, while still reducing API call frequency.
Key Changes
lastSyncTimeto track when the last sync occurred and calculate remaining time until next syncImplementation Details
SYNC_INTERVAL = 30000)Benefits