-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
[core] Add a UnixEpochTimestampGenerator #656
Conversation
@@ -0,0 +1,188 @@ | |||
/** | |||
* Copyright (c) 2016 Yahoo! Inc. All rights reserved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of Yahoo! Inc. This should be Copyright (c) 2016 YCSB contributors. All rights reserved.
Same goes for the test file.
Fixed up the copyrights, thanks! |
Would you mind updating the commit message to start with the module impacted? In this case it'd be something like "[core] ..." |
*/ | ||
public class UnixEpochTimestampGenerator extends Generator<Long> { | ||
/** Name and default value for the timestamp interval property. */ | ||
public static final String TIMESTAMP_INTERVAL_PROPERTY = "timestamp_interval"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where are these properties getting used? What are they for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They'll be used by the TimeseriesWorkload class once it's ready. I figured it'd be good to stash them here as they could be used by multiple workloads. Or I could just move them there if you think that's better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other workloads might want to have multiple timestamps, which would then make these properties confusing. Probably best to keep them in the TimeseriesWorkload.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, yanked those guys out and I'll toss 'em in with the TS workload.
Commit message updated, thanks! |
[core] Add a UnixEpochTimestampGenerator
thanks! |
Tied to generating time series workloads: #653