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.

Sign up for GitHub

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

Jump to bottom

Redirect Get message by id request when broker not serve for the topic #7786

Merged
merged 3 commits into from Aug 17, 2020
Merged

Redirect Get message by id request when broker not serve for the topic #7786

merged 3 commits into from Aug 17, 2020

Conversation

rudy2steiner
Copy link
Contributor

@rudy2steiner rudy2steiner commented Aug 9, 2020 •
edited

(If this PR fixes a github issue, please add Fixes #<xyz>.)

Fixes #7604

Master Issue: #7604

Motivation

We should redirect Get message by id request when a broker doesn't serve for the topic

Modifications

  • Redirect Get message by id request when a broker doesn't serve for the topic
  • Test log config use log4j2.xml not logback.xml
    config by parent pom: -Dlog4j.configurationFile=log4j2.xml

Verifying this change

This change is a trivial changes without any test coverage.

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): ( no)
  • The public API: (yes )
  • The schema: (no)
  • The default values of configurations: (no)
  • The wire protocol: (no)
  • The rest endpoints: ( no)
  • The admin cli options: (no)
  • Anything that affects deployment: (no)

sijie
sijie reviewed Aug 10, 2020
View reviewed changes
pulsar-broker/src/test/resources/log4j2.xml Outdated
@@ -19,18 +19,16 @@

-->
<configuration scan="true">
<!--
Copy link
Member

Choose a reason for hiding this comment

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

The change doesn't seem to be related.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I try to debug the API, using LoadBalancerTest to launch a multi nodes cluster and trying to change the debug level, such as org.apache.pulsar.broker.admin. but logback.xml configuration doesn't work and found that we actually using log4j2.xml which is configured in parent pom.

I just change the log config file by the way, it's ok if you still think we shouldn't change this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

revert

@rudy2steiner
Copy link
Contributor Author

@sijie please help review !

@sijie
Copy link
Member

sijie commented Aug 11, 2020

/pulsarbot run-failure-checks

@sijie sijie added this to the 2.7.0 milestone Aug 11, 2020
@rudy2steiner
Copy link
Contributor Author

I will check the unit test exception,seem like not cause by this changes:

org.testng.internal.thread.ThreadTimeoutException: Method org.apache.pulsar.client.api.SimpleProducerConsumerTest.testSharedSamePriorityConsumer() didn't finish within the time-out 5000

@codelipenghui
Copy link
Contributor

/pulsarbot run-failure-checks

@rudy2steiner
Copy link
Contributor Author

/pulsarbot run-failure-checks

2 similar comments
@rudy2steiner
Copy link
Contributor Author

/pulsarbot run-failure-checks

@rudy2steiner
Copy link
Contributor Author

/pulsarbot run-failure-checks

@rudy2steiner
Copy link
Contributor Author

I try to run the same unit test command on local

./build/retry.sh mvn -B -ntp test -pl pulsar-broker -Dinclude='org/apache/pulsar/broker/**/*.java' -Dexclude='org/apache/pulsar/broker/zookeeper/**/*.java,org/apache/pulsar/broker/loadbalance/**/*.java,org/apache/pulsar/broker/service/**/*.java,**/AdminApiOffloadTest.java'

without any failed test case.
But online test failed after retry 3 times and each time failed with different test cases:

[ERROR] Failures: 
[ERROR] org.apache.pulsar.broker.transaction.coordinator.TransactionMetaStoreAssignmentTest.testTransactionMetaStoreAssignAndFailover(org.apache.pulsar.broker.transaction.coordinator.TransactionMetaStoreAssignmentTest)
[INFO]   Run 1: PASS
[ERROR]   Run 2: TransactionMetaStoreAssignmentTest.testTransactionMetaStoreAssignAndFailover:45 expected [16] but found [0]
[INFO] 
[INFO]
-----------
[ERROR] Failures: 
[ERROR] org.apache.pulsar.broker.admin.IncrementPartitionsTest.testIncrementPartitionsWithNoSubscriptions(org.apache.pulsar.broker.admin.IncrementPartitionsTest)
[INFO]   Run 1: PASS
[ERROR]   Run 2: IncrementPartitionsTest.testIncrementPartitionsWithNoSubscriptions:130 expected [20] but found [10]
[INFO] 
[ERROR]   TransactionProduceTest.setup ? PulsarAdmin java.util.concurrent.CompletionExce...
[INFO] 
[ERROR] Tests run: 417, Failures: 2, Errors: 0, Skipped: 2

-----
[ERROR] Failures: 
[ERROR] org.apache.pulsar.broker.admin.IncrementPartitionsTest.testIncrementPartitionsWithNoSubscriptions(org.apache.pulsar.broker.admin.IncrementPartitionsTest)
[INFO]   Run 1: PASS
[ERROR]   Run 2: IncrementPartitionsTest.testIncrementPartitionsWithNoSubscriptions:127 expected [10] but found [2]
[INFO] 
[ERROR] org.apache.pulsar.broker.admin.MaxUnackedMessagesTest.testMaxUnackedMessagesOnSubscriptionApi(org.apache.pulsar.broker.admin.MaxUnackedMessagesTest)
[INFO]   Run 1: PASS
[ERROR]   Run 2: MaxUnackedMessagesTest.testMaxUnackedMessagesOnSubscriptionApi ? ServerSideError
[INFO] 
[ERROR] org.apache.pulsar.broker.transaction.coordinator.TransactionMetaStoreAssignmentTest.testTransactionMetaStoreAssignAndFailover(org.apache.pulsar.broker.transaction.coordinator.TransactionMetaStoreAssignmentTest)
[INFO]   Run 1: PASS
[ERROR]   Run 2: TransactionMetaStoreAssignmentTest.testTransactionMetaStoreAssignAndFailover:45 expected [16] but found [0]
[INFO] 
[INFO] 
[ERROR] Tests run: 415, Failures: 3, Errors: 0, Skipped: 0

@sijie sijie merged commit 6580915 into apache:master Aug 17, 2020
huangdx0726 pushed a commit to huangdx0726/pulsar that referenced this pull request Aug 24, 2020
@rudy2steiner @huangdx0726
Redirect Get message by id request when broker not serve for the topic (
28f5284 
apache#7786)

Fixes apache#7604

Master Issue: apache#7604

### Motivation

We should redirect Get message by id request when a broker doesn't serve for the topic

### Modifications

* Redirect Get message by id request when a broker doesn't serve for the topic
* ~~Test log config use log4j2.xml not logback.xml
     config by parent pom: -Dlog4j.configurationFile=log4j2.xml~~

### Verifying this change

This change is a trivial change without any test coverage.
lbenc135 pushed a commit to lbenc135/pulsar that referenced this pull request Sep 5, 2020
@rudy2steiner
Redirect Get message by id request when broker not serve for the topic (
a50c0a3 
apache#7786)

Fixes apache#7604

Master Issue: apache#7604

### Motivation

We should redirect Get message by id request when a broker doesn't serve for the topic

### Modifications

* Redirect Get message by id request when a broker doesn't serve for the topic
* ~~Test log config use log4j2.xml not logback.xml
     config by parent pom: -Dlog4j.configurationFile=log4j2.xml~~

### Verifying this change

This change is a trivial change without any test coverage.
lbenc135 pushed a commit to lbenc135/pulsar that referenced this pull request Sep 5, 2020
@rudy2steiner
Redirect Get message by id request when broker not serve for the topic (
151bc1c 
apache#7786)

Fixes apache#7604

Master Issue: apache#7604

### Motivation

We should redirect Get message by id request when a broker doesn't serve for the topic

### Modifications

* Redirect Get message by id request when a broker doesn't serve for the topic
* ~~Test log config use log4j2.xml not logback.xml
     config by parent pom: -Dlog4j.configurationFile=log4j2.xml~~

### Verifying this change

This change is a trivial change without any test coverage.
lbenc135 pushed a commit to lbenc135/pulsar that referenced this pull request Sep 5, 2020
@rudy2steiner
Redirect Get message by id request when broker not serve for the topic (
dff6668 
apache#7786)

Fixes apache#7604

Master Issue: apache#7604

### Motivation

We should redirect Get message by id request when a broker doesn't serve for the topic

### Modifications

* Redirect Get message by id request when a broker doesn't serve for the topic
* ~~Test log config use log4j2.xml not logback.xml
     config by parent pom: -Dlog4j.configurationFile=log4j2.xml~~

### Verifying this change

This change is a trivial change without any test coverage.
wolfstudy pushed a commit that referenced this pull request Oct 30, 2020
@rudy2steiner @wolfstudy
Redirect Get message by id request when broker not serve for the topic (
3de632b 
#7786)

Fixes #7604

Master Issue: #7604

### Motivation

We should redirect Get message by id request when a broker doesn't serve for the topic

### Modifications

* Redirect Get message by id request when a broker doesn't serve for the topic
* ~~Test log config use log4j2.xml not logback.xml
     config by parent pom: -Dlog4j.configurationFile=log4j2.xml~~

### Verifying this change

This change is a trivial change without any test coverage.

(cherry picked from commit 6580915)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sijie sijie sijie approved these changes

@codelipenghui codelipenghui codelipenghui approved these changes

Assignees

@rudy2steiner rudy2steiner

Labels
area/broker release/2.6.2
Projects
None yet
Milestone
2.7.0
Development

Successfully merging this pull request may close these issues.

[Rest admin api] 'Get message by its messageId' throw exception when the broker asked is not the good one
3 participants
@rudy2steiner @sijie @codelipenghui