[trafficserver] branch master updated: Allow binding value to non-nil variable in LuaVM for configuration files evaluations. This allows us to fix issue #2511.

[ki...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

kichan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/master by this push:
     new 326cd98  Allow binding value to non-nil variable in LuaVM for configuration files evaluations. This allows us to fix issue #2511.
326cd98 is described below

commit 326cd980d64e0e8cabd788f71f25c135061bc573
Author: Kit Chan <ki...@apache.org>
AuthorDate: Fri Oct 13 01:07:49 2017 -0700

    Allow binding value to non-nil variable in LuaVM for configuration files evaluations. This allows us to fix issue #2511.
    
    The problem is that the "avg" and "10s" metrics rely on the current timestamp continuously being evaluated and binded to
    a constant inside the Lua VM. However, there are mechanism currently prevent the new timestamp to be bind to a constant
    that was previously binded already. Therefore the timestamp is never getting updated in the lua VM.
---
 cmd/traffic_manager/test_metrics.cc | 37 +++++++++++++++++++++++++++++++++++
 lib/bindings/bindings.cc            | 39 +++++++------------------------------
 2 files changed, 44 insertions(+), 32 deletions(-)

diff --git a/cmd/traffic_manager/test_metrics.cc b/cmd/traffic_manager/test_metrics.cc
index 52ca96e..84b2a42 100644
--- a/cmd/traffic_manager/test_metrics.cc
+++ b/cmd/traffic_manager/test_metrics.cc
@@ -68,6 +68,43 @@ integer 'proxy.node.test.value' [[
   metrics_binding_destroy(binding);
 }
 
+// Check that we can bind a constant and set the value based on that.
+// Also check that we can bind the constant again and get the updated value.
+REGRESSION_TEST(EvalBoundedMetrics)(RegressionTest *t, int /* atype ATS_UNUSED */, int *pstatus)
+{
+  TestBox box(t, pstatus);
+
+  box = REGRESSION_TEST_PASSED;
+
+  const char *config = R"(
+integer 'proxy.node.test.value' [[
+  return test
+]]
+  )";
+
+  BindingInstance binding;
+
+  box.check(metrics_binding_initialize(binding), "initialize metrics");
+  box.check(binding.bind_constant("test", 100), "binding for a constant");
+  box.check(binding.eval(config), "load metrics config");
+
+  metrics_binding_evaluate(binding);
+
+  RecInt value = 0;
+  box.check(RecGetRecordInt("proxy.node.test.value", &value) == REC_ERR_OKAY, "read value (100) from proxy.node.test.value");
+  box.check(value == 100, "proxy.node.test.value was %" PRId64 ", wanted 100", value);
+
+  box.check(binding.bind_constant("test", 101), "update binding for the constant");
+
+  metrics_binding_evaluate(binding);
+
+  value = 0;
+  box.check(RecGetRecordInt("proxy.node.test.value", &value) == REC_ERR_OKAY, "read value (101) from proxy.node.test.value");
+  box.check(value == 101, "proxy.node.test.value was %" PRId64 ", wanted 101", value);
+
+  metrics_binding_destroy(binding);
+}
+
 int
 main(int argc, const char **argv)
 {
diff --git a/lib/bindings/bindings.cc b/lib/bindings/bindings.cc
index a50e951..3178352 100644
--- a/lib/bindings/bindings.cc
+++ b/lib/bindings/bindings.cc
@@ -98,7 +98,6 @@ BindingInstance::bind_value(const char *name, int value)
 {
   const char *start = name;
   const char *end   = name;
-  bool bound        = false;
 
   int depth = 0;
 
@@ -174,48 +173,24 @@ BindingInstance::bind_value(const char *name, int value)
   // If we pushed a series of tables onto the stack, bind the name to a table
   // entry. otherwise bind it as a global name.
   if (depth) {
-    bool isnil;
-
     // At this point the top of stack should be something indexable.
     ink_assert(is_indexable(this->lua, -1));
 
     Debug("lua", "stack depth is %d (expected %d)", lua_gettop(this->lua), depth);
-    // Push the index name.
-    lua_pushstring(this->lua, start);
-
-    Debug("lua", "stack depth is %d (expected %d)", lua_gettop(this->lua), depth);
-    // Fetch the index (without metamethods);
-    lua_gettable(this->lua, -2);
 
-    // Only push the value if it is currently nil.
-    isnil = lua_isnil(this->lua, -1);
-    lua_pop(this->lua, 1);
-    Debug("lua", "isnil? %s", isnil ? "yes" : "no");
-
-    if (isnil) {
-      lua_pushstring(this->lua, start);
-      lua_pushvalue(this->lua, value);
-      lua_settable(this->lua, -3);
-      bound = true;
-    }
+    lua_pushstring(this->lua, start);
+    lua_pushvalue(this->lua, value);
+    lua_settable(this->lua, -3);
 
     Debug("lua", "stack depth is %d (expected %d)", lua_gettop(this->lua), depth);
     lua_pop(this->lua, depth);
   } else {
-    bool isnil;
-
-    lua_getglobal(this->lua, start);
-    isnil = lua_isnil(this->lua, -1);
-    lua_pop(this->lua, 1);
-
-    if (isnil) {
-      lua_pushvalue(this->lua, value);
-      lua_setglobal(this->lua, start);
-      bound = true;
-    }
+    // Always push the value so we can get the update
+    lua_pushvalue(this->lua, value);
+    lua_setglobal(this->lua, start);
   }
 
-  return bound;
+  return true;
 }
 
 bool

-- 
To stop receiving notification emails like this one, please contact
['"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>'].