Decrement commitBatchesMemBytesCount when rejecting commit batch release-6.3

[sfc-gh-nwijetunga]

This issue relates to: #4523

We were never decrementing commitBatchesMemBytesCount when rejecting a commit batch causing this value to "blow up". This PR address this to make sure we are decrementing before returning. This is a backport of #4525 which is needed in 6.3 because this code path can get triggered causing commits to get clogged

This PR passes 10k Joshua runs

Code-Reviewer Section

The general guidelines can be found here.

Please check each of the following things and check all boxes before accepting a PR.

• The PR has a description, explaining both the problem and the solution.
• The description mentions which forms of testing were done and the testing seems reasonable.
• Every function/class/actor that was touched is reasonably well documented.

For Release-Branches

If this PR is made against a release-branch, please also check the following:

• This change/bugfix is a cherry-pick from the next younger branch (younger release-branch or master if this is the youngest branch)
• There is a good reason why this PR needs to go into a release branch and this reason is documented (either in the description above or in a linked GitHub issue)

[jzhou77]

LGTM

[xumengpanda]

As the new release process states, changes should go against master branch before it is made on release branch.

Out of curiosity, how did you find the bug? What kind of tests did you run? Can we include the test as well, if it's simulation test?

[xumengpanda]

It seems simulation may be able to catch the bug if we increase the probability of exercising the code path: e.g., BUGGIFY_WITH_PROB(0.01) -> BUGGIFY_WITH_PROB(0.1) at
https://github.com/apple/foundationdb/blob/554667466a25348f81c710579c4b2c2fbf6abbbc/fdbserver/MasterProxyServer.actor.cpp#L1007

[sfc-gh-nwijetunga]

As the new release process states, changes should go against master branch before it is made on release branch.

Out of curiosity, how did you find the bug? What kind of tests did you run? Can we include the test as well, if it's simulation test?

I have already made the pr for master (linked it in the description but it's #4525). As for the test it was a Watches.actor.cpp workload failure that caught this, more detail is added in the issue (#4523) which is linked above. Since simulation already hits this I don't think there is a need to increase the Buggify probability

[sfc-gh-abeamon]

I have already made the pr for master

I guess one thing that's a bit confusing about this process is that you'll also need to update the 6.3 release notes for this on master (currently, master has a copy of all old releases notes including for 6.3).

[sfc-gh-nwijetunga]

I have already made the pr for master

I guess one thing that's a bit confusing about this process is that you'll also need to update the 6.3 release notes for this on master (currently, master has a copy of all old releases notes including for 6.3).

Oh okay was not aware of this, will make a pr against master once I merge in both the 6.2 and 6.3 release branch fixes