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

[Broker] Failed to process metadata store notification #11637

Closed
lhotari opened this issue Aug 11, 2021 · 8 comments
Closed

[Broker] Failed to process metadata store notification #11637

lhotari opened this issue Aug 11, 2021 · 8 comments
Labels
type/bug The PR fixed a bug or issue reported a bug

Comments

@lhotari
Copy link
Member

lhotari commented Aug 11, 2021

Describe the bug

I came a cross multiple odd log entries "Failed to process metadata store notification
java.lang.IllegalArgumentException: Invalid namespace format. expected / or // but got: public" when investigating #11635 .

I wonder if this has recently changed. This is the log entry:

06:28:50.626 [metadata-store-16-1] INFO  org.apache.pulsar.common.naming.NamespaceBundleFactory - Policy updated for namespace public/dedup-2, refreshing the bundle cache.
06:28:50.626 [metadata-store-16-1] ERROR org.apache.pulsar.metadata.impl.AbstractMetadataStore - Failed to process metadata store notification
java.lang.IllegalArgumentException: Invalid namespace format. expected <tenant>/<namespace> or <tenant>/<cluster>/<namespace> but got: public
	at org.apache.pulsar.common.naming.NamespaceName.<init>(NamespaceName.java:125) ~[org.apache.pulsar-pulsar-common-2.9.0-SNAPSHOT.jar:2.9.0-SNAPSHOT]
	at org.apache.pulsar.common.naming.NamespaceName.<init>(NamespaceName.java:34) ~[org.apache.pulsar-pulsar-common-2.9.0-SNAPSHOT.jar:2.9.0-SNAPSHOT]
	at org.apache.pulsar.common.naming.NamespaceName$1.load(NamespaceName.java:46) ~[org.apache.pulsar-pulsar-common-2.9.0-SNAPSHOT.jar:2.9.0-SNAPSHOT]
	at org.apache.pulsar.common.naming.NamespaceName$1.load(NamespaceName.java:43) ~[org.apache.pulsar-pulsar-common-2.9.0-SNAPSHOT.jar:2.9.0-SNAPSHOT]
	at com.google.common.cache.LocalCache$LoadingValueReference.loadFuture(LocalCache.java:3529) ~[com.google.guava-guava-30.1-jre.jar:?]
	at com.google.common.cache.LocalCache$Segment.loadSync(LocalCache.java:2278) ~[com.google.guava-guava-30.1-jre.jar:?]
	at com.google.common.cache.LocalCache$Segment.lockedGetOrLoad(LocalCache.java:2155) ~[com.google.guava-guava-30.1-jre.jar:?]
	at com.google.common.cache.LocalCache$Segment.get(LocalCache.java:2045) ~[com.google.guava-guava-30.1-jre.jar:?]
	at com.google.common.cache.LocalCache.get(LocalCache.java:3951) ~[com.google.guava-guava-30.1-jre.jar:?]
	at com.google.common.cache.LocalCache.getOrLoad(LocalCache.java:3974) ~[com.google.guava-guava-30.1-jre.jar:?]
	at com.google.common.cache.LocalCache$LocalLoadingCache.get(LocalCache.java:4935) ~[com.google.guava-guava-30.1-jre.jar:?]
	at org.apache.pulsar.common.naming.NamespaceName.get(NamespaceName.java:69) ~[org.apache.pulsar-pulsar-common-2.9.0-SNAPSHOT.jar:2.9.0-SNAPSHOT]
	at org.apache.pulsar.common.naming.NamespaceBundleFactory.handleMetadataStoreNotification(NamespaceBundleFactory.java:162) ~[org.apache.pulsar-pulsar-broker-2.9.0-SNAPSHOT.jar:2.9.0-SNAPSHOT]
	at org.apache.pulsar.metadata.impl.AbstractMetadataStore.lambda$receivedNotification$0(AbstractMetadataStore.java:149) ~[org.apache.pulsar-pulsar-metadata-2.9.0-SNAPSHOT.jar:2.9.0-SNAPSHOT]
	at java.util.concurrent.CopyOnWriteArrayList.forEach(CopyOnWriteArrayList.java:895) ~[?:1.8.0_292]
	at org.apache.pulsar.metadata.impl.AbstractMetadataStore.lambda$receivedNotification$1(AbstractMetadataStore.java:147) ~[org.apache.pulsar-pulsar-metadata-2.9.0-SNAPSHOT.jar:2.9.0-SNAPSHOT]
	at java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1604) [?:1.8.0_292]
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) [?:1.8.0_292]
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) [?:1.8.0_292]
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) [io.netty-netty-common-4.1.66.Final.jar:4.1.66.Final]
	at java.lang.Thread.run(Thread.java:748) [?:1.8.0_292]
Caused by: java.lang.IllegalArgumentException: Invalid namespace format. namespace: public
	at org.apache.pulsar.common.naming.NamespaceName.<init>(NamespaceName.java:122) ~[org.apache.pulsar-pulsar-common-2.9.0-SNAPSHOT.jar:2.9.0-SNAPSHOT]
	... 20 more

To Reproduce

The above example is from the logs of running cpp-tests (see #11635). I assume that this happens in the master branch for all namespace policy updates.

Expected behavior

Namespace policy updates shouldn't result in errors.

@lhotari lhotari added the type/bug The PR fixed a bug or issue reported a bug label Aug 11, 2021
@lhotari
Copy link
Member Author

lhotari commented Aug 11, 2021

@merlimat This problem might be related to recent PIP-45 changes. Please take a look.

@lhotari
Copy link
Member Author

lhotari commented Aug 11, 2021

this seems to always fail:

private void handleMetadataStoreNotification(Notification n) {
if (n.getPath().startsWith(LOCAL_POLICIES_ROOT)) {
final NamespaceName namespace = NamespaceName.get(getNamespaceFromPoliciesPath(n.getPath()));
try {
LOG.info("Policy updated for namespace {}, refreshing the bundle cache.", namespace);
// Trigger a background refresh to fetch new bundle data from the policies
bundlesCache.synchronous().invalidate(namespace);
} catch (Exception e) {
LOG.error("Failed to update the policy change for ns {}", namespace, e);
}
}
}

this logic looks risky:

public static String getNamespaceFromPoliciesPath(String path) {
if (path.isEmpty()) {
return path;
}
// String before / is considered empty string by splitter
Iterable<String> splitter = Splitter.on("/").limit(6).split(path);
Iterator<String> i = splitter.iterator();
// skip first three - "","admin", "policies"
i.next();
i.next();
i.next();
// prop, cluster, namespace
return Joiner.on("/").join(i);
}

@eolivelli
Copy link
Contributor

I am pretty sure that CI passed when I merged the #11198 patch

probably there is some case that happens rarely

@lhotari
Copy link
Member Author

lhotari commented Aug 11, 2021

I am pretty sure that CI passed when I merged the #11198 patch

probably there is some case that happens rarely

It looks like a problem that just gets logged. The problem seems to be that NotificationType.ChildrenChanged isn't handled in NamespaceBundleFactory.handleMetadataStoreNotification.

@eolivelli
Copy link
Contributor

I see.
let's fix it

@lhotari
Copy link
Member Author

lhotari commented Aug 15, 2021

This problem seems to be fixed by #11656 . @Technoboy- can you confirm?

@Technoboy-
Copy link
Contributor

This problem seems to be fixed by #11656 . @Technoboy- can you confirm?

Yes, I think so.

@lhotari
Copy link
Member Author

lhotari commented Aug 16, 2021

Closing since this is fixed by #11656

@lhotari lhotari closed this as completed Aug 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

No branches or pull requests

3 participants