add last-delivered-id to StreamGroupInfo by AndyPook · Pull Request #1477 · StackExchange/StackExchange.Redis (original) (raw)

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

@AndyPook

"last-delivered-id" was added to XINFO GROUPS in the 5.0 timeframe.
It can be useful as a proxy for stream lag when compared to StreamInfo,LastGeneratedId
However, the redis-doc wasn't updated (see redis/redis-doc#1318)

This PR simply adds the field to the StreamGroupInfo response.

mgravell

Choose a reason for hiding this comment

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

Merging, but I'm also going to change the code (not just yours) to not rely on position - not sure that is reliable

@mgravell

mgravell added a commit that referenced this pull request

May 29, 2020

@mgravell

@AndyPook

thanks

I was going to have a look at the positional stuff. redis-doc does say that clients shouldn't rely on ordering.
But if you're already on it

@AndyPook

Maybe something like

    public static class SeqEx
    {
        public static RedisValue[] Get(this Sequence<RedisValue> values, params string[] names)
        {
            if (names.Length * 2 != values.Length)
                throw new ArgumentException("values should be exactly double the length of names");

            var result = new RedisValue[names.Length];
            for (int i = 0; i < values.Length; i += 2)
            {
                if (values[i] == names[i])
                    result[i] = values[i + 1];
            }

            return result;
        }
    }

The result will be guaranteed to be in the order of the names supplied.
Or it could be added next to the GetItems method in RawResult?

The nice thing would be that none of the result types need to change. It's just that the ctor calls in ResultProcessor would need their indexes changed to be sequential. A fairly big, if just tedious, change.

Just an idea :)

@mgravell

That pretty much just retains the positional version, but at least checks the names. I'd rather not depend on the order.

…

On Sat, 30 May 2020, 14:21 Andy Pook, ***@***.***> wrote: Maybe something like public static class SeqEx { public static RedisValue[] Get(this Sequence values, params string[] names) { if (names.Length * 2 != values.Length) throw new ArgumentException("values should be exactly double the length of names"); var result = new RedisValue[names.Length]; for (int i = 0; i < values.Length; i += 2) { if (values[i] == names[i]) result[i] = values[i + 1]; } return result; } } The result will be guaranteed to be in the order of the names supplied. Or it could be added next to the GetItems method in RawResult? The nice thing would be that none of the result types need to change. It's just that the ctor calls in ResultProcessor would need their indexes changed to be sequential. A fairly big, if just tedious, change. Just an idea :) — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#1477 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAEHMFWCPTOXEDQ4E7O25LRUEB4XANCNFSM4NNFU5TA> .

@AndyPook

You are right, I'm an idiot :)

    public static RedisValue Get(this RedisValue[] values, string name)
    {
        for (int i = 0; i < values.Length; i += 2)
        {
            if (values[i] == name)
                return values[i + 1];
        }
        return RedisValue.EmptyString;
    }

    public static RedisValue[] GetValues(this RedisValue[] values, params string[] names)
    {
        if (names.Length * 2 != values.Length)
            throw new ArgumentException("values should be exactly double the length of names");

        var result = new RedisValue[names.Length];
        for (int i = 0; i < values.Length; i += 2)
        {
            for (int j = 0; j < names.Length; j++)
            {
                if (names[j]!=null && values[i] == names[j])
                {
                    result[j] = values[i + 1];
                    names[j] = null;
                }
            }
        }

        return result;
    }

Get is the obvious approach.
GetValues works, in that the result is in the order given by the names.
I'm not sure I like it. The null bit is an attempt at making the loop faster. But maybe better than mapping to a dictionary?
Any approach is going to cost. But this is probably not going to be on the hi-perf path. So maybe it doesn't matter?

Although redis-doc says the order is undefined, from the code, it is constant for a given release.

@mgravell

Did you see the commit (I cross-reference it, it appears above) where I already implemented this at the RawResult level, so it doesn't need to create any RedisValue instances or the array?

(RawResult is the tokenized inbound data stream in a zero-copy way; CommandBytes is a hacked-up way of expressing and testing literals without allocations)

@AndyPook

Ahh, sorry missed that, been spending too long in AzDevOps.
Cool, learned another something new today :)

As all these bits are internal, I've got a hacked up method using .ExecuteAsync("XINFO" ...
I am "not allowed" to use pre-release packages. When do you think this would get a nuget stable release?

@mgravell

I need to revisit a possible glitch in the mutex code before I can do that; have my hands busy with 16 other things at the moment, but: it is on my list!

2 participants

@AndyPook @mgravell