gnmi-server: cache search stops immediately if error happens in any of the subscription-cache

[fahadnaeemkhan]

oc_cache stores data per subscriptions so targets can be spread across multiple subscriptions (depending on configuration) and when we try to subscribes, it looks for match in all the subscriptions and create one goroutine per subscription for finding/query:

https://github.com/openconfig/gnmic/blob/e1e58b86eb33f50038c2393a260cbbfa666b3099/pkg/cache/oc_cache.go#L186

func (gc *gnmiCache) handleSingleQuery(ctx context.Context, ro *ReadOpts, ch chan *Notification) {
    if gc.debug {
        gc.logger.Printf("running single query for target %q", ro.Target)
    }

    caches := gc.getCaches(ro.Subscription)

    if gc.debug {
        gc.logger.Printf("single query got %d caches", len(caches))
    }
    wg := new(sync.WaitGroup)
    wg.Add(len(caches))

    for name, c := range caches {
        go func(name string, c *subCache) {
            defer wg.Done()
            for _, p := range ro.Paths {
                fp, err := path.CompletePath(p, nil)
                if err != nil {
                    gc.logger.Printf("failed to generate CompletePath from %v", p)
                    ch <- &Notification{Name: name, Err: err}
                    return
                }
                err = c.c.Query(ro.Target, fp,
                    func(_ []string, l *ctree.Leaf, _ interface{}) error {
                        if err != nil {
                            return err
                        } 
                                                 ....

Issue here is that these query goroutines sends errors to channel which gnmi_server is reading from and incase of cache miss from any of the goroutines, error is send to gnmi_server which returns immediately after seeing error even though data might me present under another subscription. https://github.com/openconfig/gnmic/blob/e1e58b86eb33f50038c2393a260cbbfa666b3099/pkg/app/gnmi_server.go#L683

There are multiple solutions here:

• These query go routines doesn't have to send errors directly to the channel from which gnmi_server is reading from. We can have a separate channel b/w subscription goroutine and the query goroutines for errors. Subscription goroutine can collect all the errors and from here we could send error to gnmi_server after going through all the errors.
• Have a shared flag say cache_hit b/w all the query go routines and if any of the goroutines finds the data, they can set the flag to true. Once all the query go routines are done, check the flag, if its unset then sends the error on the channel to gnmi_server. We can use https://pkg.go.dev/sync#Once for setting the flag.
• Another solution can be not to return but just log the error in gnmi_server, this will allow other query goroutines to send the notifications if available

Let me know if this is making sense or more info is needed

[karimra]

While your description of how the oc_cache works is correct, the subscription does not fail in case data does not exist in the cache. The subscribe function fails in these cases:

• One of the paths is malformed.
• No target specified (* counts as wildcard).
• The specified target does not exist in one of the subscriptions caches.
• The callback function given to the query returns an error.

The only error that could count as a cache miss here is the 3rd one (The specified target does not exist in one of the subscriptions caches). All the other errors should result in a failure of the subscription.

The case of missing target for one of the subscription scaches can be solved by calling cache.HasTarget() before running the query. Let me know if you want to give this a shot in a PR.

[fahadnaeemkhan]

Yeah I was trying to refer to case 3.

Sure, would love to submit a PR for this.

[fahadnaeemkhan]

PR submitted: https://github.com/openconfig/gnmic/pull/288