Skip to content

Conversation

@ajbeamon
Copy link
Contributor

@ajbeamon ajbeamon commented Jul 8, 2019

Add a random UID to TransactionMetrics in case a client opens multiple connections and also a field to indicate whether the connection is internal. Convert some of the metrics to our Counter object instead of running totals.

…e connections and also a field to indicate whether the connection is internal. Convert some of the metrics to our Counter object instead of running totals.
@ajbeamon ajbeamon added this to the 6.2 milestone Jul 8, 2019
@ajbeamon ajbeamon requested a review from xumengpanda July 8, 2019 21:08
@ajbeamon ajbeamon changed the title Add a random UID to TransactionMetrics in case a client opens multiple… Improve TransactionMetrics Jul 8, 2019
@AlvinMooreSr
Copy link
Contributor

@fdb-build test windows please

int64_t transactionsProcessBehind;
int64_t transactionWaitsForFullRecovery;
UID dbId;
bool internal;
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel it is better to add a comment behind internal to clarify: what request are considered as internal.
The comment can be:
// true if request is from FDB components, e.g., backup and restore.

It can save time when someone reads the code one year later.

@xumengpanda
Copy link
Contributor

Not sure why CTest fails. I'm throwing it to correctness test and let's see.

@xumengpanda
Copy link
Contributor

Correctness fails sadly.
One error case is: -r simulation --logsize 1024MiB -f foundationdb/tests/fast/LongStackWriteDuringRead.txt -b on -s 327764438

Copy link
Contributor

@xumengpanda xumengpanda left a comment

Choose a reason for hiding this comment

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

Correctness reports segmentation faults.
One failed test case is:
-r simulation --logsize 1024MiB -f foundationdb/tests/fast/LongStackWriteDuringRead.txt -b on -s 327764438

@ajbeamon
Copy link
Contributor Author

ajbeamon commented Jul 9, 2019

Darn, this is the result an editor issue that led to some changes from another branch being left over. Fix coming shortly.

@xumengpanda xumengpanda self-requested a review July 9, 2019 23:45
@xumengpanda xumengpanda merged commit cce00bb into apple:master Jul 9, 2019
@xumengpanda
Copy link
Contributor

The problem has been fixed. It passes 20k random tests.

@ajbeamon ajbeamon deleted the improved-transaction-metrics branch July 10, 2019 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants