@sundb do you see any reason not to delete the two other increments to server.dirty in this function?

Ths, These two increments really seem unnecessary.

@itamarhaber @guybe7 please confirm that i'm not missing anything.
not sure why in some places we used to increment the dirty with the amount of changed fields, and in others just increment once for the changed key.

@oranagra
I've organized some case:

1. add, update, delete N times with or without dbDelete, server.dirty += N
2. dbDelete only, no add, update, delete, server.dirty += 1
3. dbAdd and insert, server.dirty += 1

Sorry, I can't confirm although it looks ok. I believe, in the sense that's my current understanding, that as long as dirty is larger than the number of keys modified, it should be ok... but belief isn't something you build software with :)

@sundb one last thing before i merge this, did you go over all the commands that change multiple elements, and these were the only two that needed such a fix?

@sundb one last thing before i merge this, did you go over all the commands that change multiple elements, and these were the only two that needed such a fix?

These days in the hospital, and so home I went through all the codes.

@sundb I hope all is well with you and yours.

@itamarhaber Thanks.

@oranagra I have gone through all the code that involves server.dirty modification, I think there should be ok.

I think it might be better to unify, in the future.
There is a case i can tell.

We use list as a MQ in some simple scenarios. Like:

pipeline.lrange('log:stat:xxxx_log', 0, args.chunk - 1)
pipeline.ltrim('log:stat:xxxx_log', args.chunk, -1)

The changes in server.dirty += (ltrim + rtrim);
Will cause the trigger of bgsave to not meet my expectations.
(Will be easily triggered by server_cron)

Because I always thought that changes in save are about keys.... in redis.conf we can see:

# Unless specified otherwise, by default Redis will save the DB:
#   * After 3600 seconds (an hour) if at least 1 key changed
#   * After 300 seconds (5 minutes) if at least 100 keys changed
#   * After 60 seconds if at least 10000 keys changed
#
# You can set these explicitly by uncommenting the three following lines.
#
# save 3600 1
# save 300 100
# save 60 10000

Although this seems reasonable, the list has indeed changed so many times.
And We may also want to persist for this situation

I'm not 100% certain what's the right thing and what was the intention.
I think it is likely that the documentation got outdated (left there from a time there were no complex data types).
on the other hand, some of the pre-existing lines like server.dirty += count in 9feee42 are very explicit.
that's why we assumed that the other places that incremented just once where simply overlooked.

# Unless specified otherwise, by default Redis will save the DB:
#   * After 3600 seconds (an hour) if at least 1 key changed
#   * After 300 seconds (5 minutes) if at least 100 keys changed
#   * After 60 seconds if at least 10000 keys changed

i want to fix the outdated doc (about the key word)
I thought about using elements, but it is also not right, because in FLUSHDB, server.dirty += dictSize(db)
Not to mention ++ in some other places. so maybe change it to at least 1 change from the last save

and i suppose we can also mention the rdb_changes_since_last_save (user can see it in INFO command).

long long dirty; /* Changes to DB from the last save */

i agree, the word "key" is outdated.
maybe "if at least 1 change was performed"?