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

Partitioned consumer test failure #323

Merged
merged 6 commits into from
Mar 31, 2017

Conversation

jai1
Copy link
Contributor

@jai1 jai1 commented Mar 30, 2017

Motivation

Reduce the number of intermittent test failures before we start work on #238

Modifications

  • Fixed a few test cases

Result

  • lesser number of intermittent build failures in CPP client

@jai1 jai1 added this to the 1.17 milestone Mar 30, 2017
@jai1 jai1 self-assigned this Mar 30, 2017
@jai1 jai1 requested review from rdhabalia and saandrews March 30, 2017 08:17
boost::bind(resendMessage, _1, _2, producer));
}
} else {
producer.sendAsync(MessageBuilder().setProperty("attempt#", boost::lexical_cast<std::string>(attemptNumber + 1)).build(),
boost::bind(resendMessage, _1, _2, producer));
Copy link
Contributor

Choose a reason for hiding this comment

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

can't we just write

if (attemptNumber < 3) {
    globalResendMessageCount++;
    lock.unlock();
}
producer.sendAsync(MessageBuilder().setProperty("attempt#", boost::lexical_cast<std::string>(attemptNumber + 1)).build(),
boost::bind(resendMessage, _1, _2, producer));

@@ -815,6 +826,6 @@ TEST(BasicEndToEndTest, testMessageListenerPause)

// 3 seconds
usleep(3 * 1000 * 1000);

Lock lock(mutex_);
Copy link
Contributor

Choose a reason for hiding this comment

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

What was the failure without the lock?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are two threads using the variable - hence W/O the lock the behavior is undefined.

Copy link
Contributor

@merlimat merlimat left a comment

Choose a reason for hiding this comment

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

👍

@merlimat merlimat modified the milestones: 1.17, 1.18 Mar 31, 2017
@jai1 jai1 merged commit a7d476f into apache:master Mar 31, 2017
@jai1 jai1 deleted the PartitionedConsumerTestFailure branch March 31, 2017 04:55
hrsakai pushed a commit to hrsakai/pulsar that referenced this pull request Dec 10, 2020
…#323)

### Motivation
enable users to configure the logger used by the client and use their own implementation. If no logger is provided, a wrapped `logrus.StandardLogger()` will be used. <s>This PR only solved part of the problem mentioned in the issue https://github.com/apache/pulsar-client-go/issues/228.</s>

### Modifications

* define `Logger` and `Entry` interfaces used by the client
* add `Logger` field to ClientOptions
* add `logger` field to internal structures
* provide a logger implementation backed by logrus
* implement a no-op logger
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