Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix][flaky-test]ConsumedLedgersTrimTest #17116

Merged

Conversation

poorbarcode
Copy link
Contributor

@poorbarcode poorbarcode commented Aug 16, 2022

Fixes

Master Issue: #11145 #10380 #17044

Motivation

restartBroker();
// force load topic
pulsar.getAdminClient().topics().getStats(topicName);
MessageId messageIdAfterRestart = pulsar.getAdminClient().topics().getLastMessageId(topicName);
LOG.info("lastmessageid " + messageIdAfterRestart);
assertEquals(messageIdAfterRestart, messageIdBeforeRestart);

In addition to calling internalTrimLedgers directly in this unit test, the Broker also has scheduled tasks consumedLedgersMonitor calling internalTrimLedgers.

(High light)See the code above, If consumedLedgersMonitor call internalTrimLedgers before line:148:
the command topics.getLastMessageId will return -1:-1:-1, then the #11145 #10380 occurs.

persistentTopic = (PersistentTopic) pulsar.getBrokerService().getOrCreateTopic(topicName).get();
managedLedgerConfig = persistentTopic.getManagedLedger().getConfig();
managedLedgerConfig.setRetentionSizeInMB(-1);
managedLedgerConfig.setRetentionTime(1, TimeUnit.SECONDS);
managedLedgerConfig.setMaxEntriesPerLedger(1);
managedLedgerConfig.setMinimumRolloverTime(1, TimeUnit.MILLISECONDS);
managedLedger = (ManagedLedgerImpl) persistentTopic.getManagedLedger();
// now we have two ledgers, the first is expired but is contains the lastMessageId
// the second is empty and should be kept as it is the current tail
Assert.assertEquals(managedLedger.getLedgersInfoAsList().size(), 2);

(High light)See the code above, If consumedLedgersMonitor call internalTrimLedgers before line:161:
the command managedLedger.getLedgersInfoAsList().size() will return 1, then the #17044 occurs.

Modifications

  • Fix flaky test: Make internalTrimLedgers never trigger by consumedLedgersMonitor before consumedLedgersMonitor is triggered manually: disabled policy retention.

for (int i = 0; i < msgNum; i++) {
Message<byte[]> msg = consumer.receive(2, TimeUnit.SECONDS);
assertNotNull(msg);
consumer.acknowledge(msg);
}
Assert.assertEquals(managedLedger.getLedgersInfoAsList().size(), msgNum / 2);

  • Try to solve another hidden Falky test: See the code above, I think line:99 may be flaky, because we can't guarantee that consumedLedgersMonitor --> internalTrimLedgers will be executed later than line:99, so delete line:99

Documentation

  • doc-required
    (Your PR needs to update docs and you will update later)

  • doc-not-needed
    (Please explain why)

  • doc
    (Your PR contains doc changes)

  • doc-complete
    (Docs have been already added)

@poorbarcode
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

@poorbarcode
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

@poorbarcode
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

@poorbarcode
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

2 similar comments
@poorbarcode
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

@poorbarcode
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

@poorbarcode poorbarcode force-pushed the flaky/ConsumedLedgersTrimTest branch from a7514a2 to 8f3f1b5 Compare August 18, 2022 09:16
@codelipenghui codelipenghui merged commit f6fd6b7 into apache:master Aug 18, 2022
@Technoboy- Technoboy- modified the milestones: 2.12.0, 2.11.0 Aug 19, 2022
Jason918 pushed a commit that referenced this pull request Sep 4, 2022
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Sep 16, 2022
(cherry picked from commit f6fd6b7)
(cherry picked from commit fef3b4f)
@poorbarcode poorbarcode deleted the flaky/ConsumedLedgersTrimTest branch September 17, 2022 02:45
congbobo184 pushed a commit that referenced this pull request Nov 8, 2022
@congbobo184 congbobo184 added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Nov 8, 2022
congbobo184 pushed a commit that referenced this pull request Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment