public orgId -1 -> 0 and related cleanups #880
Conversation
Dieterbe
force-pushed
the
explicit-orgidpublic
branch
from
14d0406 to
2446a0a
Compare
|
this is step 1 to address #260 |
idx/memory/memory.go
Outdated
| @@ -1190,7 +1192,7 @@ func (m *MemoryIdx) delete(orgId int, n *Node, deleteEmptyParents, deleteChildre | |||
| func (m *MemoryIdx) Prune(orgId int, oldest time.Time) ([]idx.Archive, error) { | |||
| oldestUnix := oldest.Unix() | |||
| orgs := make(map[int]struct{}) | |||
| if orgId == -1 { | |||
| if orgId == idx.OrgIdPublic { | |||
There was a problem hiding this comment.
Doesnt look like the use of -1 had anything to do with the PublicOrg. So i think instead this should be
if orgId == 0 {
idx/memory/memory.go
Outdated
| @@ -1300,7 +1302,7 @@ DEFS: | |||
|
|
|||
| statMetricsActive.Add(-1 * len(pruned)) | |||
|
|
|||
| if orgId == -1 { | |||
| if orgId == idx.OrgIdPublic { | |||
There was a problem hiding this comment.
Doesnt look like the use of -1 had anything to do with the PublicOrg. So i think instead this should be
if orgId == 0 {
idx/cassandra/cassandra.go
Outdated
| @@ -504,7 +504,7 @@ func (c *CasIdx) prune() { | |||
| for range ticker.C { | |||
| log.Debug("cassandra-idx: pruning items from index that have not been seen for %s", maxStale.String()) | |||
| staleTs := time.Now().Add(maxStale * -1) | |||
| _, err := c.Prune(-1, staleTs) | |||
| _, err := c.Prune(idx.OrgIdPublic, staleTs) | |||
There was a problem hiding this comment.
Doesnt look like the use of -1 had anything to do with the PublicOrg. So i think instead this should be
_, err := c.Prune(0, staleTs)
Dieterbe
force-pushed
the
explicit-orgidpublic
branch
from
2446a0a to
4bd2355
Compare
there is never a need to list data across ALL orgs in one request
this will allow us to use uint encoding for org id's fix #260
Dieterbe
force-pushed
the
explicit-orgidpublic
branch
from
4bd2355 to
5db1111
Compare
Dieterbe
changed the title
|
@woodsaj PTAL |
| @@ -13,6 +13,8 @@ import ( | |||
| schema "gopkg.in/raintank/schema.v1" | |||
| ) | |||
|
|
|||
| const OrgIdPublic = 0 | |||
There was a problem hiding this comment.
shouldn't we just make this configurable with a command line arg?
There was a problem hiding this comment.
what's the benefit of that?
I think this should be a constant across the entire stack and the surrounding ecosystem of tools:
org 0 = public data
org 1 = admin org
org >1 = private tenant orgs
making it configurable seems to cause nothing but complications.
There was a problem hiding this comment.
Why does the public org need to be 0. Wasnt the whole point of using a variable (or const) to remove the need to have special values.
For the Wolrdping case, we currently have to hard code the collection of the public data into the probe. But we could simplify everything by just creating a public org and configure the endpoints for it, known that the data collected for that org will be viewable by all users.
There was a problem hiding this comment.
Wasnt the whole point of using a variable (or const) to remove the need to have special values.
No, the point was to only use orgid's that are >=0 so that they are encodeable as a uint. this is also what was explained in the original ticket #260
there's nothing wrong with special reserved values per se. I think conventions are useful, as long as they are simple and consistently applied. they simplify things (less config flags to worry about and keep in sync across different software, clearer code)
Your proposed simplification for WP probes sounds nice, but does it really conflict with the proposed convention? is it not possible to just create that public org with id 0 ?
There was a problem hiding this comment.
I dont really like using the zero value, as it then makes it impossible to know if the value was set or not.
But as we dont have a use case for needing non-zero right now, lets just leave this. It will be trivial to change this later if we want/need to.
|
@Dieterbe LGTM, if there is any use cases for the public org in the TSDB-GW that you want implemented let me know and I can add it to the refactor |
MemoryIdx.Findif -1 was passed previously it would do the same search twice and then discard the results from the second run.