Skip to content

Conversation

@vazarkevych
Copy link
Collaborator

@vazarkevych vazarkevych commented Aug 18, 2025

In this pr we:

  • Added FileStickyBucketServiceImpl that implements StickyBucketService using GbCacheManager to persist all sticky bucket assignments in a single file.
  • Supports asynchronous fetching and saving via CompletableFuture.

Copy link
Contributor

@madhuchavva madhuchavva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vazarkevych Thoughtful contribution on providing File-based storage. But it involves a lot of risks that are needed to be taken care such as Concurrency bottlenecks (read-modify-write), Performance issues (full file read/write per operation) and Corruption risks (if the process crashes midway, the data is lost and the file isn't readable anymore).

On top of it, This PR replaces the current synchronous behavior for StickyBucketingService interface with async methods, which breaks the existing/custom implementations and concurrent access to a shared file needs synchronization.

Can you provide more details about this feature request?

@vazarkevych
Copy link
Collaborator Author

@vazarkevych Thoughtful contribution on providing File-based storage. But it involves a lot of risks that are needed to be taken care such as Concurrency bottlenecks (read-modify-write), Performance issues (full file read/write per operation) and Corruption risks (if the process crashes midway, the data is lost and the file isn't readable anymore).

On top of it, This PR replaces the current synchronous behavior for StickyBucketingService interface with async methods, which breaks the existing/custom implementations and concurrent access to a shared file needs synchronization.

Can you provide more details about this feature request?

Thank you for the review!
I fully agree with the concerns regarding interface consistency and the risks associated with file I/O.
I’ve made substantial changes to address all the issues raised, while still preserving the purpose of the PR - providing a safe file-based storage.

1. Contract Violation - resolved

Changing StickyBucketService to an asynchronous contract (CompletableFuture) was indeed a mistake, as it introduced a breaking change.

Fix:
The StickyBucketService interface has been fully restored to its original synchronous form (returning StickyAssignmentsDocument or Map<...>). This maintains backward compatibility with all existing implementations.

2. File Storage Risks & Concurrency

I improved FileStickyBucketServiceImpl to minimize concurrency issues and corruption risks:

✔️ Concurrency

To avoid race conditions during file-level "read-modify-write" operations, I added key-based locking using a ConcurrentHashMap.
This ensures that read/write operations for the same file are thread-safe and properly synchronized.

✔️ Safety & Integrity

The implementation now operates within a synchronized block to ensure controlled access to the file system.

3. 💡 Feature Request Background

This request originated from community needs analysis, particularly similar to the Swift SDK (Issue #116), where asynchronous support for StickyBucketService was added. This is needed when users want to avoid blocking the main application thread during slow I/O operations. Also, the TypeScript SDK's base StickyBucketService is also fundamentally asynchronous (returning Promises) to natively support remote persistence solutions

Although in this PR we reverted to a synchronous interface for compatibility, the FileStickyBucketServiceImpl implementation shows how I/O-based storage can be used safely in a synchronous environment.

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