IGNITE-27178 Add a total number of unresolved write intents as a metric #7428
Conversation
|
|
||
| @Test | ||
| void pendingWriteIntentsMetric() { | ||
| String tabName = "test_table_pending_wi"; |
There was a problem hiding this comment.
We don't usually use short "abbreviations" like tab in Ignite 3. Ignite 2 and Ignite 3 have different code style guidelines, please keep that in mind.
| tx.commit(); | ||
| } | ||
|
|
||
| expectPendingWriteIntents(tabName, 0); |
There was a problem hiding this comment.
Please run this test several hundred or thousand times. I'm afraid that it might be unstable. Write intents cleanup is an asynchronous process, as far as I know, and here you're just hoping that it is completed between tx.commit() and expectPendingWriteIntents(tabName, 0), which is not guaranteed.
I think you should use awaitility to wait until this method stops throwing assertions. Not sure about the next one, please check if there are any transitive guarantees for modsCount value. Maybe there aren't any
There was a problem hiding this comment.
expectPendingWriteIntents is a custom method which does use Awaitility.
| public int getPendingRowAmount() { | ||
| return pendingRows.getPendingRowAmount(); | ||
| } |
There was a problem hiding this comment.
I believe it's Count, not Amount. Rows are countable, you can say "3 rows" and it'll be grammatically correct. I'd also use Rows, not Row
There was a problem hiding this comment.
Indeed, the count is much better. Regarding rows vs row I would disagree. English compound nouns keep the modifier singular (user count, item count, row count).
|
|
||
| storageUpdateHandler.start(onNodeRecovery); | ||
|
|
||
| registerPartitionModificationCounterMetrics(table, partitionId, modificationCounter, storageUpdateHandler::getPendingRowAmount); |
There was a problem hiding this comment.
Please rename this method as well, so that it matches the name of metric source
Lalant
force-pushed
the
ignite-27178-pending-write-intent-metrics
branch
from
a21a33c to
7260bc6
Compare
Lalant
force-pushed
the
ignite-27178-pending-write-intent-metrics
branch
from
7260bc6 to
8d0d138
Compare
2 participants
https://issues.apache.org/jira/projects/IGNITE/issues/IGNITE-27178