Skip to content

Conversation

@codelipenghui
Copy link
Contributor

@codelipenghui codelipenghui commented May 17, 2022

Motivation

#14839 fixes the HTTP lookup with multiple advertised listeners, and the added test
is trying to verify the advertised listeners with 2 brokers, but the config of the
the additional broker will not update, so the test gets passed.

The root cause is we are using the (advertised address + WebService port) to register
the broker under the loadbalance path, but while creating the lookup result, we will
use the WebService URL to get the data, so we will get MetadataStoreException$NotFoundException.

2022-05-17T14:18:55,658+0800 [pulsar-load-manager-38-1] INFO  org.apache.pulsar.metadata.coordination.impl.ResourceLockImpl - Acquired resource lock on /loadbalance/brokers/BROKER-0:15004
2022-05-17T14:19:01,126+0800 [pulsar-4-1] WARN  org.apache.pulsar.broker.lookup.TopicLookupBase - Failed to lookup broker for topic persistent://public/default/my-topic: org.apache.pulsar.metadata.api.MetadataStoreException$NotFoundException: /loadbalance/brokers/xxxx:15004
java.util.concurrent.CompletionException: org.apache.pulsar.metadata.api.MetadataStoreException$NotFoundException: /loadbalance/brokers/xxxx:15004
	at java.util.concurrent.CompletableFuture.encodeThrowable(CompletableFuture.java:332) ~[?:?]
	at java.util.concurrent.CompletableFuture.uniAcceptNow(CompletableFuture.java:747) ~[?:?]
	at java.util.concurrent.CompletableFuture.uniAcceptStage(CompletableFuture.java:735) ~[?:?]
	at java.util.concurrent.CompletableFuture.thenAccept(CompletableFuture.java:2182) ~[?:?]
	at org.apache.pulsar.broker.namespace.NamespaceService.searchForCandidateBroker(NamespaceService.java:599) ~[classes/:?]
	at org.apache.pulsar.broker.namespace.NamespaceService.lambda$findBrokerServiceUrl$8(NamespaceService.java:405) ~[classes/:?]
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539) [?:?]
	at java.util.concurrent.FutureTask.run$$$capture(FutureTask.java:264) [?:?]
	at java.util.concurrent.FutureTask.run(FutureTask.java) [?:?]
	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304) [?:?]
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136) [?:?]
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) [?:?]
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) [netty-common-4.1.76.Final.jar:4.1.76.Final]
	at java.lang.Thread.run(Thread.java:833) [?:?]
Caused by: org.apache.pulsar.metadata.api.MetadataStoreException$NotFoundException: /loadbalance/brokers/xxxx:15004
	at org.apache.pulsar.broker.namespace.NamespaceService.lambda$createLookupResult$18(NamespaceService.java:642) ~[classes/:?]
	at java.util.concurrent.CompletableFuture.uniAcceptNow(CompletableFuture.java:757) ~[?:?]
	at java.util.concurrent.CompletableFuture.uniAcceptStage(CompletableFuture.java:735) ~[?:?]
	at java.util.concurrent.CompletableFuture.thenAccept(CompletableFuture.java:2182) ~[?:?]
	at org.apache.pulsar.broker.namespace.NamespaceService.createLookupResult(NamespaceService.java:620) ~[classes/:?]
	at org.apache.pulsar.broker.namespace.NamespaceService$MockitoMock$1465504970.createLookupResult$accessor$rPJehdnB(Unknown Source) ~[classes/:?]
	at org.apache.pulsar.broker.namespace.NamespaceService$MockitoMock$1465504970$auxiliary$608QTNMe.call(Unknown Source) ~[classes/:?]
	at org.mockito.internal.invocation.RealMethod$FromCallable$1.call(RealMethod.java:40) ~[mockito-core-3.12.4.jar:?]
	at org.mockito.internal.invocation.RealMethod$FromBehavior.invoke(RealMethod.java:62) ~[mockito-core-3.12.4.jar:?]
	at org.mockito.internal.invocation.InterceptedInvocation.callRealMethod(InterceptedInvocation.java:142) ~[mockito-core-3.12.4.jar:?]
	at org.mockito.internal.stubbing.answers.CallsRealMethods.answer(CallsRealMethods.java:45) ~[mockito-core-3.12.4.jar:?]
	at org.mockito.Answers.answer(Answers.java:99) ~[mockito-core-3.12.4.jar:?]
	at org.mockito.internal.handler.MockHandlerImpl.handle(MockHandlerImpl.java:110) ~[mockito-core-3.12.4.jar:?]
	at org.mockito.internal.handler.NullResultGuardian.handle(NullResultGuardian.java:29) ~[mockito-core-3.12.4.jar:?]
	at org.mockito.internal.handler.InvocationNotifierHandler.handle(InvocationNotifierHandler.java:34) ~[mockito-core-3.12.4.jar:?]
	at org.mockito.internal.creation.bytebuddy.MockMethodInterceptor.doIntercept(MockMethodInterceptor.java:82) ~[mockito-core-3.12.4.jar:?]
	at org.mockito.internal.creation.bytebuddy.MockMethodInterceptor.doIntercept(MockMethodInterceptor.java:56) ~[mockito-core-3.12.4.jar:?]
	at org.mockito.internal.creation.bytebuddy.MockMethodInterceptor$DispatcherDefaultingToRealMethod.interceptSuperCallable(MockMethodInterceptor.java:145) ~[mockito-core-3.12.4.jar:?]
	at org.apache.pulsar.broker.namespace.NamespaceService$MockitoMock$1465504970.createLookupResult(Unknown Source) ~[classes/:?]
	at org.apache.pulsar.broker.namespace.NamespaceService.searchForCandidateBroker(NamespaceService.java:598) ~[classes/:?]
	... 9 more

Modification

  • Added properties support for ResourceUnit, so that we can carry more information while getting the least loaded broker
  • For creating the lookup result, use the broker znode name to get the broker data if it presents in the ResourceUnit

Verification

No new tests are needed, just fix the test that was added in #14839

Documentation

Check the box below or label this PR directly.

Need to update docs?

  • no-need-doc
    (bug fix)

…g topic lookup

### Motivation

apache#14839 fixes the http lookup with multiple advertised listeners, and the added test
is try to verify the advertised listeners with 2 brokers, but the config of the
additional broker will not update, so the test get passed.

The root cause is we are using the (advertised address + WebService port) to register
the broker under the loadbalance path, but while creating the lookup result, we will
use the WebService URL to get the data, so we will get `MetadataStoreException$NotFoundException`.

### Modification

- Added properties support for ResourceUnit, so that we can carry more information while getting least loaded broker
- For creating the lookup result, use the broker znode name to get the broker data if it presents in the ResourceUnit

### Verification

No new tests needed, just fix the test that added in apache#14839
@codelipenghui codelipenghui requested a review from merlimat May 17, 2022 09:45
@codelipenghui codelipenghui self-assigned this May 17, 2022
@codelipenghui codelipenghui added type/bug The PR fixed a bug or issue reported a bug area/broker labels May 17, 2022
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label May 17, 2022
@codelipenghui codelipenghui added release/2.9.0 release/2.10.1 and removed doc-not-needed Your PR changes do not impact docs labels May 17, 2022
@github-actions
Copy link

@codelipenghui:Thanks for your contribution. For this PR, do we need to update docs?
(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

1 similar comment
@github-actions
Copy link

@codelipenghui:Thanks for your contribution. For this PR, do we need to update docs?
(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

@codelipenghui codelipenghui added the doc-not-needed Your PR changes do not impact docs label May 17, 2022
@codelipenghui codelipenghui added this to the 2.11.0 milestone May 17, 2022
@github-actions
Copy link

@codelipenghui:Thanks for providing doc info!

@codelipenghui
Copy link
Contributor Author

/pulsarbot run-failure-checks

@Technoboy- Technoboy- merged commit 70551a6 into apache:master May 19, 2022
@codelipenghui codelipenghui deleted the penghui/fix-broker-lookup branch May 20, 2022 07:05
codelipenghui added a commit that referenced this pull request May 20, 2022
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request May 23, 2022
…g topic lookup (apache#15633)

(cherry picked from commit 70551a6)
(cherry picked from commit 6415b31)
mattisonchao pushed a commit that referenced this pull request May 25, 2022
@mattisonchao mattisonchao added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/broker cherry-picked/branch-2.9 Archived: 2.9 is end of life cherry-picked/branch-2.10 doc-not-needed Your PR changes do not impact docs release/2.9.3 release/2.10.1 type/bug The PR fixed a bug or issue reported a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants