Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change RedisTimeSeries resolution from milliseconds to microseconds #1465

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jansopenyld
Copy link

Milliseconds resolution is not enough today for fast computers.
In many financial data the granularity of data is higher than milliseconds.

The Redis time series has only millisecond time resolution. The timestamps are
expected to be UNIX timestamp in milliseconds. That resolution is not good
enough for high-frequency data, for example trading data.

This change updates the TS. commands so that the commands accepts as timestamp
microseconds instead.

Since the timestamps as numbers by large passing timestamp as microsecond
timestamps just works (64 bit integers used as timestamps are able to hold the
timestamps).

This change just adds a cosmetic changes, so that TS.ADD with timestamp * adds a
timestamp in microseconds, instead of milliseconds.

Also retention policy code has been updated to convert retention to
microseconds.

As art of this change, the Redis SDK was upgraded to version 7.0. This is needed
since method Redis_microseconds() is available only since version 7.0.
@jansopenyld jansopenyld requested a review from danni-m as a code owner May 15, 2023 23:26
@CLAassistant
Copy link

CLAassistant commented May 15, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@LiorKogan LiorKogan left a comment

Choose a reason for hiding this comment

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

This would introduce a breaking change. Obviously we can't allow it.

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.

None yet

4 participants