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.

Sign up for GitHub

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

Jump to bottom

Support all timestamp_resolution units in TWCS #16057

Open
nyh opened this issue Nov 14, 2023 · 1 comment · May be fixed by #17269
Open

Support all timestamp_resolution units in TWCS #16057

nyh opened this issue Nov 14, 2023 · 1 comment · May be fixed by #17269
Labels
area/cassandra 3.x compatibility area/cql area/twcs P4 Low Priority

Comments

@nyh
Copy link
Contributor

nyh commented Nov 14, 2023

In issue #3152 we noticed that we need to support timestamp_resolution = MILLISECONDS in TWCS for KairosDB that uses it (the default is MICROSECONDS). We did this in commit 0c72781.

Cassandra actually allows several other units, not just MICROSECONDS or MILLISECONDS, and although nobody is likely to ever use them (I don't think we ever had complaints from users), we should consider supporting all the units and not just two of them.

Noticed this in #16027 - we had a Scylla test that checks that "SECONDS" isn't supported as timestamp_resolution - but then we noticed that it failed on Cassandra because it is supported on Cassandra.
@nyh nyh mentioned this issue Nov 14, 2023
Closed
@nyh nyh added the area/cql label Nov 14, 2023
andriipatsula added a commit to andriipatsula/scylla that referenced this issue Feb 11, 2024
@andriipatsula
sstables/twcs: add support for seconds, minutes, hours, and days time…
e84acdf 
…stamp resolutions.

Cassandra supports timestamp resolutions beyond MICROSECONDS and MILLISECONDS for the Time Window Compaction Strategy (all units understandable by Java TimeUnit). To ensure compatibility, we extend the list of supported timestamp resolutions to include seconds, minutes, hours, and days.

Fixes scylladb#16057
@andriipatsula andriipatsula linked a pull request Feb 11, 2024 that will close this issue
Open
@nyh
Copy link
Contributor Author

nyh commented Feb 12, 2024

Update from discussion in #17269: It appears that the existing code (compaction/time_window_compaction_strategy.cc) in addition to the sub-second units MILLISECONDS and MICROSECONDS, also supported the whole-second units MINUTES, HOURS and DAYS. The full lists of units we need to support is Java's https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/TimeUnit.html - so we're only missing support for SECONDS (this is the problem that started this issue) and NANOSECONDS. The rest should already be fine.

andriipatsula added a commit to andriipatsula/scylla that referenced this issue Feb 24, 2024
@andriipatsula
sstables/twcs: add support for seconds, minutes, hours, and days time…
a6c1cd7 
…stamp resolutions.

Cassandra supports timestamp resolutions beyond MICROSECONDS and MILLISECONDS for the Time Window Compaction Strategy (all units understandable by Java TimeUnit). To ensure compatibility, we extend the list of supported timestamp resolutions to include seconds, minutes, hours, and days.

Fixes scylladb#16057
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/cassandra 3.x compatibility area/cql area/twcs P4 Low Priority
Projects
None yet
Milestone
No milestone
1 participant
@nyh