[FSSDK-11513] limit number of events in the eventStore#1053
[FSSDK-11513] limit number of events in the eventStore#1053
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR limits the number of events stored in the event store to prevent excessive storage use. Key changes include:
- Adding a new log message (EVENT_STORE_FULL) for cases when the event store reaches capacity.
- Implementing event count management via eventCountInStore and a dedicated storeEvent method with logic to skip storing events when the maximum is reached.
- Updating tests to cover scenarios when the event store limit is met and when store saves fail.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lib/message/log_message.ts | Added new log message constant for full event store notification. |
| lib/event_processor/batch_event_processor.ts | Introduced event store capacity logic (eventCountInStore, storeEvent) and replaced dispatchingEventIds with dispatchingEvents. |
| lib/event_processor/batch_event_processor.spec.ts | Added tests covering event store limits and failure conditions. |
| this.eventStore?.remove(id); | ||
| events.forEach((event) => { | ||
| this.eventStore?.remove(event.id); | ||
| if (!event.notStored && this.eventCountInStore) { |
There was a problem hiding this comment.
The check 'this.eventCountInStore' may fail when the count is 0 (since 0 is falsy), which could prevent the intended decrement. Consider replacing it with 'this.eventCountInStore !== undefined' to ensure the condition works correctly even when the count is 0.
| if (!event.notStored && this.eventCountInStore) { | |
| if (!event.notStored && this.eventCountInStore !== undefined) { |
| batches.push({ | ||
| request: buildLogEvent(currentBatch.map((e) => e.event)), | ||
| ids: currentBatch.map((e) => e.id), | ||
| // ids: currentBatch.map((e) => e.id), |
There was a problem hiding this comment.
[nitpick] If the 'ids' array is no longer needed after switching to 'events', please consider removing the commented code to improve code clarity.
| // ids: currentBatch.map((e) => e.id), |
junaed-optimizely
left a comment
There was a problem hiding this comment.
LGTM! Its a nice optimization!
d3ea562 to
67d0c18
Compare
Summary
Test plan
Issues