Skip to content

Increment the stat_rdb_saves counter in SAVE command#10827

Merged
oranagra merged 2 commits intoredis:unstablefrom
enjoy-binbin:stat_rdb_saves_in_save_command
Jun 7, 2022
Merged

Increment the stat_rdb_saves counter in SAVE command#10827
oranagra merged 2 commits intoredis:unstablefrom
enjoy-binbin:stat_rdb_saves_in_save_command

Conversation

@enjoy-binbin
Copy link
Contributor

@enjoy-binbin enjoy-binbin commented Jun 7, 2022

Currently, we only increment stat_rdb_saves in rdbSaveBackground,
we should also increment it in the SAVE command.

We concluded there's no need to increment when:

  1. saving a base file for an AOF
  2. when saving an empty rdb file to delete an old one
  3. when saving to sockets (not creating a persistence / snapshot file)

The stat counter was introduced in #10178

Currently, we only increment stat_rdb_saves in rdbSaveBackground,
we should also increment it in the SAVE command.

The stat counter was introduced in redis#10178
@enjoy-binbin
Copy link
Contributor Author

posted by oran with the others one (from #10178 (comment))

  • i think there's value (or at least it's very different) setting it before we fork, rather than at completion (in which case we need to decide if we count failures too).
  • i don't think the implicit save of FLUSHALL should count, arguably it's the same as deleting the old persistence file, rather than saving an empty one.
  • i don't think we should count the empty rdb we save when creating a an empty base for a multipart AOF when starting up empty.

@oranagra oranagra merged commit ae9764e into redis:unstable Jun 7, 2022
@enjoy-binbin enjoy-binbin deleted the stat_rdb_saves_in_save_command branch June 7, 2022 14:40
enjoy-binbin added a commit to enjoy-binbin/redis that referenced this pull request Jul 31, 2023
Currently, we only increment stat_rdb_saves in rdbSaveBackground,
we should also increment it in the SAVE command.

We concluded there's no need to increment when:
1. saving a base file for an AOF
2. when saving an empty rdb file to delete an old one
3. when saving to sockets (not creating a persistence / snapshot file)

The stat counter was introduced in redis#10178

* fix a wrong comment in startSaving
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants