Skip to content
This repository has been archived by the owner on Aug 23, 2023. It is now read-only.

add metricdefs to cassandra if memory idx DID succeed #560

Merged
merged 1 commit into from
Mar 15, 2017

Conversation

Dieterbe
Copy link
Contributor

@Dieterbe Dieterbe commented Mar 8, 2017

we have 3 cases where we update cassandra idx after attempting to
update memory idx.
A) in memory && same partition, but old lastUpdate
B) in memory but different partition
C) not in memory yet

B and C were correct, but in A, via
40c2f15 in #504
the following bug was introduced:

  1. we saved defs to cassandra when the update in memory did not succeed.
    With the current code MemoryIdx.AddOrUpdateDef always returns error=nil,
    so this case could not manifest itself, although until recently
    (008737c) we returned
    idx.BranchUnderLeaf and idx.BothBranchAndLeaf errors.
    So for a while we saved defs with those errors to the cassandra index

  2. we did not update cassandra when the memory AddOrUpdate did succeed.
    The consequence here is that the lastUpdate field in cassandra was out
    of date, so that when new instances start from scratch, loading the
    index, they may have excluded metrics from render requests when they
    were supposed to be included, but only for inactive series for which it
    did not receive any new points (since when it receives points, it'll
    update the LastUpdate in memory)

we have 3 cases where we update cassandra idx after attempting to
update memory idx.
A) in memory && same partition, but old lastUpdate
B) in memory but different partition
C) not in memory yet

B and C were correct, but in A, via
40c2f15
the following bug was introduced:

1) we saved defs to cassandra when the update in memory did *not* succeed.
With the current code MemoryIdx.AddOrUpdateDef always returns error=nil,
so this case could not manifest itself, although until recently
(008737c) we returned
idx.BranchUnderLeaf and idx.BothBranchAndLeaf errors.
So for a while we saved defs with those errors to the cassandra index

2) we did not update cassandra when the memory AddOrUpdate did succeed.
The consequence here is that the lastUpdate field in cassandra was out
of date, so that when new instances start from scratch, loading the
index, they may have excluded metrics from render requests when they
were supposed to be included, but only for inactive series for which it
did not receive any new points (since when it receives points, it'll
update the LastUpdate in memory)
@replay
Copy link
Contributor

replay commented Mar 8, 2017

Nice find

@Dieterbe Dieterbe requested a review from woodsaj March 9, 2017 08:01
Copy link
Contributor

@replay replay left a comment

Choose a reason for hiding this comment

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

👍

@woodsaj
Copy link
Member

woodsaj commented Mar 15, 2017

LGTM

@Dieterbe Dieterbe merged commit a71fb93 into master Mar 15, 2017
@Dieterbe Dieterbe deleted the update-cassandra-idx-properly branch January 2, 2018 16:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants