Skip to content

Commit

Permalink
finagle-core: Add fixedinet scheme resolution to global Namer
Browse files Browse the repository at this point in the history
Problem
Name paths with fixed addresses don't need background
DNS updates. But "fixedinet" scheme was not recognized
by global namer and path like "/$/fixedinet/localhost/1234"
was not resolved.

Solution
Add additional checks for "fixedinet" scheme in InetPath
object inside global Namer.

Result
Global namer recognizes both "inet" and "fixedinet" schemes
and uses corresponding Resolver.

Differential Revision: https://phabricator.twitter.biz/D589429
  • Loading branch information
vip4f authored and jenkins committed Dec 16, 2020
1 parent b1a8ff8 commit 8798b92
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 6 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ Bug Fixes
`java.lang.UnsupportedOperationException: tail of empty stream` when a `c.t.f.s.RetryPolicy`
is converted to a String for showing. ``PHAB_ID=D582199``

* finagle-core: Recognize `fixedinet` name scheme in global `com.twitter.finagle.Namer`.
``PHAB_ID=D589429``

20.10.0
-------

Expand Down
17 changes: 11 additions & 6 deletions finagle-core/src/main/scala/com/twitter/finagle/Namer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ object Namer {

private[this] object InetPath {

private[this] def resolve(host: String, port: Int): Var[Addr] =
Resolver.eval(s"inet!$host:$port") match {
private[this] def resolve(scheme: String, host: String, port: Int): Var[Addr] =
Resolver.eval(s"$scheme!$host:$port") match {
case Name.Bound(va) => va
case n: Name.Path =>
Var.value(
Expand All @@ -82,11 +82,16 @@ object Namer {
}

def unapply(path: Path): Option[(Var[Addr], Path)] = path match {
case Path.Utf8("$", "inet", host, IntegerString(port), residual @ _*) =>
Some((resolve(host, port), Path.Utf8(residual: _*)))
case Path.Utf8("$", "inet", IntegerString(port), residual @ _*) =>
case Path.Utf8(
"$",
scheme @ ("inet" | "fixedinet"),
host,
IntegerString(port),
residual @ _*) =>
Some((resolve(scheme, host, port), Path.Utf8(residual: _*)))
case Path.Utf8("$", scheme @ ("inet" | "fixedinet"), IntegerString(port), residual @ _*) =>
// no host provided means localhost
Some((resolve("", port), Path.Utf8(residual: _*)))
Some((resolve(scheme, "", port), Path.Utf8(residual: _*)))
case _ => None
}
}
Expand Down
33 changes: 33 additions & 0 deletions finagle-core/src/test/scala/com/twitter/finagle/NamerTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,39 @@ class NamerTest extends FunSuite with AssertionsForJUnit {
}
}

test("Namer.global: /$/fixedinet") {
Namer.global.lookup(Path.read("/$/fixedinet/1234")).sample() match {
case NameTree.Leaf(Name.Bound(addr)) =>
assert(addr.sample() == Addr.Bound(Set(Address(1234))))
case _ => fail()
}

Await.result(
Namer.global.lookup(Path.read("/$/fixedinet/127.0.0.1/1234")).values.toFuture(),
1.second
)() match {
case NameTree.Leaf(Name.Bound(addr)) =>
assert(
Await.result(addr.changes.filter(_ != Addr.Pending).toFuture(), 1.second)
== Addr.Bound(Set(Address("127.0.0.1", 1234)))
)
case _ => fail()
}

intercept[ClassNotFoundException] {
Namer.global.lookup(Path.read("/$/fixedinet")).sample()
}

Namer.global.lookup(Path.read("/$/fixedinet/1234/foobar")).sample() match {
case NameTree.Leaf(bound: Name.Bound) =>
assert(bound.addr.sample() == Addr.Bound(Address(1234)))
assert(bound.id == Path.Utf8("$", "fixedinet", "1234"))
assert(bound.path == Path.Utf8("foobar"))

case _ => fail()
}
}

test("Namer.global: /$/fail") {
assert(
Namer.global.lookup(Path.read("/$/fail")).sample()
Expand Down

0 comments on commit 8798b92

Please sign in to comment.