Merge pull request #264 from ydb-platform/fix-discovery

log failed discovery, fix discovery rate on failure

Author: rekby

CHANGELOG.md

@@ -1,3 +1,6 @@
+* logged failed discovery refresh in `checkDiscovery` instead of silently swallowing the exception
+* fixed discovery retry rate after a failure: a separate `lastDiscoveryAttempt` timer gates retries by `discoveryInterval()`, so a broken discovery no longer triggers `discover()` on every API request
+
 ## 1.16.1
 * added resilient internal endpoint discovery (`YdbPlatform\Ydb\Internal\Discovery`) that always targets the original bootstrap endpoint and recreates the gRPC channel with `force_new` on retries to bust the c-core DNS cache and survive bootstrap IP changes
 * added discovery tuning config keys: `discoveryTimeoutMs` (default 1000), `discoveryAttemptTimeoutMs` (default 300), `discoveryInitialTimeoutMs` (default 5000; set to `PHP_INT_MAX` to wait indefinitely on startup)

src/Table.php

@@ -20,7 +20,7 @@
 use YdbPlatform\Ydb\Retry\Retry;
 use YdbPlatform\Ydb\Retry\RetryParams;
 
-class Table
+class  Table
 {
     use Traits\RequestTrait;
     use Traits\ParseResultTrait;

src/Traits/RequestTrait.php

@@ -37,10 +37,19 @@ trait RequestTrait
     protected $ydb;
 
     /**
-     * @var int
+     * @var int Timestamp of the last SUCCESSFUL discovery. Drives the regular
+     *          refresh schedule via discoveryInterval().
      */
     protected $lastDiscovery = 0;
 
+    /**
+     * @var int Timestamp of the last FAILED discovery attempt. Used to avoid
+     *          calling discover() on every API request while it is down — under
+     *          load each request would otherwise pay ~discoveryTimeoutMs on top
+     *          until discovery recovers.
+     */
+    protected $lastDiscoveryAttempt = 0;
+
 
 
     /**
@@ -315,13 +324,29 @@ protected function resetLastRequest()
     }
 
     protected function checkDiscovery(){
-        if ($this->ydb->needDiscovery() && time()-$this->lastDiscovery>$this->ydb->discoveryInterval()){
-            try{
-                $this->lastDiscovery = time();
-                $this->ydb->discover();
-            } catch (\Exception $e){
+        if (!$this->ydb->needDiscovery()) {
+            return;
+        }
+        $now = time();
+        $interval = $this->ydb->discoveryInterval();
+
+        // Scheduled refresh is gated by the last success; retries after a
+        // failure are gated by lastDiscoveryAttempt so a broken discovery
+        // does not get called on every single API request.
+        if ($now - $this->lastDiscovery <= $interval) {
+            return;
+        }
+        if ($now - $this->lastDiscoveryAttempt <= $interval) {
+            return;
+        }
 
-            }
+        $this->lastDiscoveryAttempt = $now;
+        try {
+            $this->logger()->debug('YDB: discovery for update ydb nodes list.');
+            $this->ydb->discover();
+            $this->lastDiscovery = $now;
+        } catch (\Exception $e) {
+            $this->logger()->warning('YDB: discovery refresh failed: ' . $e->getMessage());
         }
     }