feat: Set optional full-scan for deletion #4189
Conversation
Signed-off-by: Jiwon Park <phil.park@sktelecom.com>
|
I think this might be valuable in the interim, although you might want to chime in the discussion here as well #4133. The implicit consensus from there is to add a |
|
|
||
| for table in tables_to_delete: | ||
| self.delete_table(config, table) | ||
| if online_store_config.full_scan_for_deletion: |
There was a problem hiding this comment.
the old implementation deletes tables by default.
should full_scan_for_deletion's default value be True?
There was a problem hiding this comment.
As you said, I set default to True for compatibility with older implementations.
If compatibility is not enforced, I think False would be better.
There was a problem hiding this comment.
@breno-costa Changed to default True to ensure backward compatibility
Signed-off-by: Jiwon Park <phil.park@sktelecom.com>
What this PR does / why we need it:
In this link (#3857), I added function to perform a full-scan and delete data to save Redis memory. However, full-scan in Redis does not work beautifully, takes too long at production scale, and often fails in CI that performs feast-apply. So, I think it would be a good idea to add the 'full_scan_for_deletion' option in redis so that the user can select and delete it.
Which issue(s) this PR fixes:
Fixes