influxdata / influxdb

Scalable datastore for metrics, events, and real-time analytics
https://influxdata.com
Apache License 2.0
28.93k stars 3.55k forks source link

index in meta increased for twice #16326

Open rickif opened 4 years ago

rickif commented 4 years ago

Steps to reproduce:

influxdb 1.8, 1.7 or some other earlier versions

https://github.com/influxdata/influxdb/blob/f1d26652e91e94e050c4904818d023fe5d6701c3/services/meta/client.go

// SetData overwrites the underlying data in the meta store.
func (c *Client) SetData(data *Data) error {
    c.mu.Lock()

    // reset the index so the commit will fire a change event
    c.cacheData.Index = 0

    // increment the index to force the changed channel to fire
    d := data.Clone()
    d.Index++

    if err := c.commit(d); err != nil {
        return err
    }

    c.mu.Unlock()

    return nil
}

When func (c *Client) SetData(data *Data) error is called, the index of the data increase twice,

one for here: https://github.com/influxdata/influxdb/blob/f1d26652e91e94e050c4904818d023fe5d6701c3/services/meta/client.go#L940

another: https://github.com/influxdata/influxdb/blob/f1d26652e91e94e050c4904818d023fe5d6701c3/services/meta/client.go#L970

is that by design?

Expected behavior:

Actual behavior:

Environment info:

Config:

Logs:

Performance:

russorat commented 4 years ago

@rickif thanks for opening this. Is this causing an actual issue for you or are you just curious?

rickif commented 4 years ago

thanks for reply. I'm just curious about this when I study the source code of influxdb 1.8. I wonder whether it is by design and the purpose of the twice index increment.

sebito91 commented 4 years ago

I think you might have come across a small bug @rickif! Thanks a lot for reporting this. I'm digging into the code to see exactly what impact removing the data.Index++ inside of SetData will have (as each subsequent call to commit includes an increment).

Will get back to you shortly on this.

rickif commented 4 years ago

maybe just removing the line below will work, since the (c *Client) commit(data *Data) error is called everytime the data changed https://github.com/influxdata/influxdb/blob/f1d26652e91e94e050c4904818d023fe5d6701c3/services/meta/client.go#L940

rickif commented 4 years ago

@sebito91 I've submmitied a pull request to fix this issue. Since it's my first pr, it would be my pleasure if u could take a look

16525