Skip to content

[branch-2.10][fix][flaky-test] ManagedCursorMetricsTest.testManagedCursorMetrics - #17504

Merged
tisonkun merged 2 commits into
apache:branch-2.10from
poorbarcode:fix/flaky/cursorMetrics_2.10
Dec 9, 2022
Merged

[branch-2.10][fix][flaky-test] ManagedCursorMetricsTest.testManagedCursorMetrics#17504
tisonkun merged 2 commits into
apache:branch-2.10from
poorbarcode:fix/flaky/cursorMetrics_2.10

Conversation

@poorbarcode

@poorbarcode poorbarcode commented Sep 7, 2022

Copy link
Copy Markdown
Contributor

Fixes

Motivation & Modifications

PR #16878 cannot be cherry-picked to branch-2.10, there will be a conflict. so create this new PR.


(High light) The logic differs from #16878:

  • after cursor init: metrics brk_ml_cursor_persistLedgerSucceed is 0 in branch-master. But in current PR, it is 1

"createNewMetadataLedger" triggers once BK writes

  • in branch-2.10, "cursor.initialize" triggers the execution of "createNewMetadataLedger". see [Code in branch-2.10]
  • in branch-master, "cursor.initialize" does not triggers the execution of "createNewMetadataLedger". see [Code in branch-master]

Code in branch-2.10 (High light line: L651)

void initialize(PositionImpl position, Map<String, Long> properties, Map<String, String> cursorProperties,
final VoidCallback callback) {
recoveredCursor(position, properties, cursorProperties, null);
if (log.isDebugEnabled()) {
log.debug("[{}] Consumer {} cursor initialized with counters: consumed {} mdPos {} rdPos {}",
ledger.getName(), name, messagesConsumedCounter, markDeletePosition, readPosition);
}
createNewMetadataLedger(new VoidCallback() {
@Override
public void operationComplete() {
STATE_UPDATER.set(ManagedCursorImpl.this, State.Open);
callback.operationComplete();
}
@Override
public void operationFailed(ManagedLedgerException exception) {
callback.operationFailed(exception);
}
});
}

Code in branch-master (High light line: L653)

void initialize(PositionImpl position, Map<String, Long> properties, Map<String, String> cursorProperties,
final VoidCallback callback) {
recoveredCursor(position, properties, cursorProperties, null);
if (log.isDebugEnabled()) {
log.debug("[{}] Consumer {} cursor initialized with counters: consumed {} mdPos {} rdPos {}",
ledger.getName(), name, messagesConsumedCounter, markDeletePosition, readPosition);
}
persistPositionMetaStore(cursorLedger != null ? cursorLedger.getId() : -1L, position, properties,
new MetaStoreCallback<>() {
@Override
public void operationComplete(Void result, Stat stat) {
STATE_UPDATER.set(ManagedCursorImpl.this, State.NoLedger);
callback.operationComplete();
}
@Override
public void operationFailed(MetaStoreException e) {
callback.operationFailed(e);
}
}, false);
}

Documentation

  • doc-required

  • doc-not-needed

  • doc

  • doc-complete

@poorbarcode

Copy link
Copy Markdown
Contributor Author

/pulsarbot rerun-failure-checks

1 similar comment
@poorbarcode

Copy link
Copy Markdown
Contributor Author

/pulsarbot rerun-failure-checks

@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Sep 24, 2022
@github-actions

Copy link
Copy Markdown

The pr had no activity for 30 days, mark with Stale label.

@github-actions github-actions Bot added the Stale label Oct 25, 2022
@tisonkun tisonkun closed this Nov 14, 2022
@tisonkun tisonkun reopened this Nov 14, 2022
@tisonkun tisonkun changed the title [branch-2.10][fix][flaky-test]ManagedCursorMetricsTest.testManagedCursorMetrics [branch-2.10][fix][flaky-test] ManagedCursorMetricsTest.testManagedCursorMetrics Nov 15, 2022

@tisonkun tisonkun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for your contribution!

Merging...

@tisonkun
tisonkun merged commit 6c4e9ef into apache:branch-2.10 Dec 9, 2022
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Jan 10, 2023
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants