uid.ids-{used,available} stats #153
Conversation
| * @see UniqueId#idsAvailable() | ||
| */ | ||
| public void loadMaxId() { | ||
| this.maxId = getCurrentMaxId(); |
There was a problem hiding this comment.
Is it safe to get MAX_ID without holding the lock? The method is called from the UniqueID constructor.
There was a problem hiding this comment.
Yes it's always safe to read the current MAX_ID, because reads are always atomic in HBase.
….ids-{used,available} stats
| @@ -100,6 +100,12 @@ public TSDB(final HBaseClient client, | |||
| tag_names = new UniqueId(client, uidtable, TAG_NAME_QUAL, TAG_NAME_WIDTH); | |||
| tag_values = new UniqueId(client, uidtable, TAG_VALUE_QUAL, | |||
| TAG_VALUE_WIDTH); | |||
|
|
|||
| // Load the MAX_ID value from HBase, used for uid.{ids-used,ids-available}. | |||
There was a problem hiding this comment.
This will only load the maximum currently used IDs once. I'm not sure it's useful to load it only once, as it doesn't allow us to keep track of how IDs are getting allocated. Am I missing something?
edit: Ah OK, I see that you update the max known ID in the code path that assigns new IDs. I'm trying to decide whether this is a good idea or not... I think it's probably fine to do a get on the MAX_ID every time you want to know the max ID, which shouldn't happen too often (maybe once every 10 to 15 seconds when we self-collect our stats).
|
Thanks for the unit tests too. I added a few comments, can you please take a look and let me know what you think? Just to be clear: this won't make it in the 1.1.0 release, but in the next one. |
… getting the MAX_ID value from Hbase when collecting the stats.
|
Thank you for the feedback! Next release is fine, we've patched the RC internally to include this feature so no rush. I've changed the code to get the MAX_ID value from HBase when the stats are collected instead of maintaining a counter. When you have time please have a look and let me know what you think. Thank you! |
|
|
||
| /** The number of IDs available for future assignments; use the supplied used ID count. */ | ||
| public long idsAvailable(long idsUsed) { | ||
| return Math.max(0, (1 << idWidth * Byte.SIZE) - idsUsed - 1); |
There was a problem hiding this comment.
Can we let idsAvailable() return the total number IDs that are possible, like you did in your previous change, and not the number of IDs that are still free? Maybe we need a better name, such as maxPossibleId() or something like that.
Otherwise reporting stats ends up reading MAX_ID from HBase twice, which is unnecessary IMO.
There was a problem hiding this comment.
MAX_ID is read only once from Hbase when the stats are collected, at TSDB.java:199-201 the value fetched by idsUsed() is reused to calculate the idsAvailable(). Does this address your concern?
There was a problem hiding this comment.
Right, sorry, I was mistaken. It's actually read only once from HBase, but you need to pass it back to idsAvailable(). Wouldn't it be simpler to just return maxPossibleId() as (1 << idWidth * Byte.SIZE) - 1 and if the caller wants to know how many IDs are left, they would subtract idsUsed() from that?
There was a problem hiding this comment.
Yes, that would work. Moved the ids available calculation into the calling method in the latest commit.
ghost
mentioned this pull request
asynchronously as per isavin's work on OpenTSDB#153
…er isavin's work on OpenTSDB#153
asynchronously as per isavin's work on #153 Signed-off-by: Chris Larsen <clarsen@euphoriaaudio.com>
…er isavin's work on #153 Signed-off-by: Chris Larsen <clarsen@euphoriaaudio.com>
|
Pulled into the "next" branch with some tweaks to make a single call to HBase for all of the max UIDs (instead of one call per type) and making that call asynchronous. |
This is an attempt to address in part issue #135.
It adds two new stats (/kind):
This could be combined with the opentsdb.sh stats collector to allow you to monitor id assignment.