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

HDFS-16688. Unresolved Hosts during startup are not synced by JournalNodes #4725

Open
wants to merge 5 commits into
base: trunk
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -674,6 +674,12 @@ public class DFSConfigKeys extends CommonConfigurationKeys {
public static final boolean
DFS_NAMENODE_EDITS_QJOURNALS_RESOLUTION_ENABLED_DEFAULT = false;

public static final String
DFS_NAMENODE_EDITS_QJOURNALS_RESOLUTION_REQUIRED =
"dfs.namenode.edits.qjournals.resolution-required";
public static final boolean
DFS_NAMENODE_EDITS_QJOURNALS_RESOLUTION_REQUIRED_DEFAULT = true;

public static final String
DFS_NAMENODE_EDITS_QJOURNALS_RESOLUTION_RESOLVER_IMPL =
"dfs.namenode.edits.qjournals.resolver.impl";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,12 @@ public ListenableFuture<Long> getJournalCTime() {

@Override
public String toString() {
return InetAddresses.toAddrString(addr.getAddress()) + ':' + addr.getPort();
if (addr.isUnresolved()) {
return addr.getHostName() + ":" + addr.getPort();
snmvaughan marked this conversation as resolved.
Show resolved Hide resolved
} else {
return InetAddresses.toAddrString(addr.getAddress()) + ':' +
addr.getPort();
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,12 @@ static IPCLoggerChannelMetrics create(IPCLoggerChannel ch) {

private static String getName(IPCLoggerChannel ch) {
InetSocketAddress addr = ch.getRemoteAddress();
String addrStr = addr.getAddress().getHostAddress();
String addrStr;
if (addr.isUnresolved()) {
addrStr = addr.getHostName();
snmvaughan marked this conversation as resolved.
Show resolved Hide resolved
} else {
addrStr = addr.getAddress().getHostAddress();
}

// IPv6 addresses have colons, which aren't allowed as part of
// MBean names. Replace with '.'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,11 @@ public static List<InetSocketAddress> getAddressesList(URI uri, Configuration co
boolean resolveNeeded = conf.getBoolean(
DFSConfigKeys.DFS_NAMENODE_EDITS_QJOURNALS_RESOLUTION_ENABLED,
DFSConfigKeys.DFS_NAMENODE_EDITS_QJOURNALS_RESOLUTION_ENABLED_DEFAULT);
// If not required, the resulting address may be unresolved, but that can be corrected within
// the client once the server becomes available.
boolean resolveRequired = conf.getBoolean(
DFSConfigKeys.DFS_NAMENODE_EDITS_QJOURNALS_RESOLUTION_REQUIRED,
DFSConfigKeys.DFS_NAMENODE_EDITS_QJOURNALS_RESOLUTION_REQUIRED_DEFAULT);
DomainNameResolver dnr = DomainNameResolverFactory.newInstance(
conf,
DFSConfigKeys.DFS_NAMENODE_EDITS_QJOURNALS_RESOLUTION_RESOLVER_IMPL);
Expand All @@ -394,7 +399,7 @@ public static List<InetSocketAddress> getAddressesList(URI uri, Configuration co
// QJM should just use FQDN
String[] hostnames = dnr
.getAllResolvedHostnameByDomainName(isa.getHostName(), true);
if (hostnames.length == 0) {
if (hostnames.length == 0 && resolveRequired) {
throw new UnknownHostException(addr);
}
for (String h : hostnames) {
Expand All @@ -406,7 +411,7 @@ public static List<InetSocketAddress> getAddressesList(URI uri, Configuration co
} else {
InetSocketAddress isa = NetUtils.createSocketAddr(
addr, DFSConfigKeys.DFS_JOURNALNODE_RPC_PORT_DEFAULT);
if (isa.isUnresolved()) {
if (isa.isUnresolved() && resolveRequired) {
throw new UnknownHostException(addr);
}
addrs.add(isa);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,20 @@
<description>
Determines if the given qjournals address is a domain name which needs to
be resolved.
This is used by namenode to resolve qjournals.
This is used by namenode to resolve qjournals, and journalnode for syncing.
</description>
</property>

<property>
<name>dfs.namenode.edits.qjournals.resolution-required</name>
<value>true</value>
<description>
Determines if the given qjournals address is a domain name which is
required to be resolved.
if resolution is required, then unresolvable addresses are ignored.
if resolution is not required, then the address may be temporarily
unresolved, allowing the client to resolve it later.
This is used by namenode to resolve qjournals, and journalnode for syncing.
</description>
</property>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutorService;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.CommonConfigurationKeysPublic;
import org.apache.hadoop.hdfs.qjournal.MiniJournalCluster;
Expand All @@ -61,8 +63,6 @@

import org.apache.hadoop.util.Preconditions;
import org.apache.hadoop.thirdparty.com.google.common.collect.Maps;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.slf4j.event.Level;


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,11 @@ public void setup() throws Exception {
"testJournalNodeSyncerNotStartWhenSyncEnabled")) {
conf.set(DFSConfigKeys.DFS_NAMENODE_SHARED_EDITS_DIR_KEY,
"qjournal://jn0:9900;jn1:9901/" + journalId);
} else if (testName.getMethodName().equals(
"testJournalNodeSyncerStartWhenSyncEnabledAndUnresolved")) {
conf.set(DFSConfigKeys.DFS_NAMENODE_SHARED_EDITS_DIR_KEY,
"qjournal://jn0:9900;jn1:9901/" + journalId);
conf.setBoolean(DFSConfigKeys.DFS_NAMENODE_EDITS_QJOURNALS_RESOLUTION_REQUIRED, false);
} else if (testName.getMethodName().equals(
"testJournalNodeSyncwithFederationTypeConfigWithNameServiceId")) {
conf.set(DFSConfigKeys.DFS_NAMENODE_SHARED_EDITS_DIR_KEY +".ns1",
Expand Down Expand Up @@ -570,6 +575,12 @@ public void testJournalNodeSyncerNotStartWhenSyncEnabledIncorrectURI()

}

/**
* Require resolution. Syncer must not start when there aren't any other known journal
* nodes due to the required resolution.
*
* @throws IOException if unable to get or create the journal
*/
@Test
public void testJournalNodeSyncerNotStartWhenSyncEnabled()
throws IOException {
Expand All @@ -593,6 +604,20 @@ public void testJournalNodeSyncerNotStartWhenSyncEnabled()

}

/**
* Attempt resolution, but allow unresolved. Syncer must start because there are other potential
* journal nodes and syncing is enabled. The syncing will include the unresolved addresses in
* syncing so that once the target server is available it can be included in syncing.
*
* @throws IOException if unable to get or create the journal
*/
@Test
public void testJournalNodeSyncerStartWhenSyncEnabledAndUnresolved()
throws IOException {
jn.getOrCreateJournal(journalId);
Assert.assertEquals(true,
jn.getJournalSyncerStatus(journalId));
}

@Test
public void testJournalNodeSyncwithFederationTypeConfigWithNameServiceId()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -484,8 +484,9 @@ public void testGetJournalNodes() throws Exception {
conf.clear();
}

/*
** Test for unknown journal node host exception.
/**
* Test handling of unresolvable journal node hosts. They are still configured assuming that
* they will be resolvable in the future.
*/
@Test(expected = UnknownHostException.class, timeout = 10000)
snmvaughan marked this conversation as resolved.
Show resolved Hide resolved
public void testUnknownJournalNodeHost()
Expand Down