feat server: bring visibility to script errors #2879

adiholden · 2024-04-10T10:18:14Z

expose a metric with script errors
Add an error log of a script SHA and the reason for failure. The error log must be bounded with at most K log entries per script so we won't have logs overflow for high throughput traffic.
Additional feature (under a flag) upon receiving the undeclared variable error - transform the script to globally-atomic and retry the call.

Signed-off-by: adi_holden <adi@dragonflydb.io>

dranikpg

Please note that this is very risky behavior because it can leave the data inconsistent without notice. I think this is more serious than your system not running. Many script just don't assume they can fail. Some of them actually declare the queue name, but access arbitrary data as undeclared

romange · 2024-04-10T11:29:49Z

src/server/script_mgr.cc

@@ -33,8 +33,12 @@ ABSL_FLAG(
    bool, lua_auto_async, false,
    "If enabled, call/pcall with discarded values are automatically replaced with acall/apcall.");

-namespace dfly {
+ABSL_FLAG(bool, lua_allow_undeclared_auto_correct, true,


Let's flip it to false to avoid inconsistent behavior by default

romange · 2024-04-10T11:34:16Z

src/server/server_family.cc

@@ -2040,6 +2040,7 @@ void ServerFamily::Info(CmdArgList args, ConnectionContext* cntx) {
    append("blocked_on_interpreter", m.coordinator_stats.blocked_on_interpreter);
    append("ram_hits", m.events.ram_hits);
    append("ram_misses", m.events.ram_misses);
+    append("script_erros", script_mgr_->script_errors());


we already have "script_errors" in "info all". see should_enter("ERRORSTATS", true) section.
the goal is actually to expose script errors via /metrics

needs more fixes

Signed-off-by: adi_holden <adi@dragonflydb.io>

romange · 2024-04-11T05:40:11Z

src/facade/facade_types.h

@@ -95,6 +95,7 @@ struct ReplyStats {
  size_t io_write_cnt = 0;
  size_t io_write_bytes = 0;
  absl::flat_hash_map<std::string, uint64_t> err_count;
+  absl::flat_hash_map<std::string, std::string> script_error_map;


please add a comment saying what it maps to what

Signed-off-by: adi_holden <adi@dragonflydb.io>

romange · 2024-04-11T05:45:03Z

src/server/server_family.cc

@@ -1146,6 +1146,17 @@ void PrintPrometheusMetrics(const Metrics& m, StringResponse* resp) {
    }
  }

+  {
+    const auto& reply_stats = m.facade_stats.reply_stats;
+    string sript_error_type_metric;


Sorry, I think it's unnecessary detailed. The goal is to be able to get alerted about servers that have an error and then go to the server logs and check the warnings. So I think a simple script_error counter (not GAUGE) should do the job here.

Signed-off-by: adi_holden <adi@dragonflydb.io>

…nfly ( v1.16.1 → v1.17.0 ) (#3473) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [docker.dragonflydb.io/dragonflydb/dragonfly](https://togithub.com/dragonflydb/dragonfly) | minor | `v1.16.1` -> `v1.17.0` | --- ### Release Notes <details> <summary>dragonflydb/dragonfly (docker.dragonflydb.io/dragonflydb/dragonfly)</summary> ### [`v1.17.0`](https://togithub.com/dragonflydb/dragonfly/releases/tag/v1.17.0) [Compare Source](https://togithub.com/dragonflydb/dragonfly/compare/v1.16.1...v1.17.0) ##### Dragonfly v1.17.0 Some prominent changes include: - Improved performance for MGET operations ([#2453](https://togithub.com/dragonflydb/dragonfly/issues/2453)) - Fix argument parsing in json.objkeys ([#2872](https://togithub.com/dragonflydb/dragonfly/issues/2872)) - Fix ipv6 support for replication ([#2889](https://togithub.com/dragonflydb/dragonfly/issues/2889)) - Support serialisation of bloom filters - saving to and loading from snapshots ([#2846](https://togithub.com/dragonflydb/dragonfly/issues/2846)) - Support of HLL PFADD ([#2761](https://togithub.com/dragonflydb/dragonfly/issues/2761)) - Support bullmq workloads that do not have `{}` hashtags in their queue names ([#2890](https://togithub.com/dragonflydb/dragonfly/issues/2890)) ##### What's Changed - fix: [#2745](https://togithub.com/dragonflydb/dragonfly/issues/2745) don't start migration process again after apply the same the same config is applied by [@BorysTheDev](https://togithub.com/BorysTheDev) in [dragonflydb/dragonfly#2822 - feat(transaction): Idempotent callbacks (immediate runs) by [@dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2453 - refactor(cluster): replace sync_id with node_id for slot migration [#2835](https://togithub.com/dragonflydb/dragonfly/issues/2835) by [@BorysTheDev](https://togithub.com/BorysTheDev) in [dragonflydb/dragonfly#2838 - feat(tiering): Simple OpManager by [@dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2781 - chore: implement path mutation for JsonFlat by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2805 - feat(cluster): add migration removing by config [#2835](https://togithub.com/dragonflydb/dragonfly/issues/2835) by [@BorysTheDev](https://togithub.com/BorysTheDev) in [dragonflydb/dragonfly#2844 - chore: expose direct API on Bloom objects by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2845 - chore: generalize CompactObject::AllocateMR by [@kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2847 - feat(tiering): Simplest small bins by [@dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2810 - refactor: clean cluster slot migration code by [@BorysTheDev](https://togithub.com/BorysTheDev) in [dragonflydb/dragonfly#2848 - fix(tests): Fix numsub test by [@dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2852 - fix: healthcheck for docker containers by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2853 - fix: possible crash in tls code by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2854 - fix(server): Do not block admin-port commands by [@chakaz](https://togithub.com/chakaz) in [dragonflydb/dragonfly#2842 - fix(pytest): make pytests fail if server crash on shutdown by [@adiholden](https://togithub.com/adiholden) in [dragonflydb/dragonfly#2827 - feat(server): add prints on takeover timeout by [@adiholden](https://togithub.com/adiholden) in [dragonflydb/dragonfly#2856 - fix(pytest): dont check process return code on kill by [@adiholden](https://togithub.com/adiholden) in [dragonflydb/dragonfly#2862 - fix: authorize the http connection to call commands by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2863 - feat(cluster): Send number of keys for incoming and outgoing migrations. by [@chakaz](https://togithub.com/chakaz) in [dragonflydb/dragonfly#2858 - feat(tiering): TieredStorageV2 by [@dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2849 - bug(server): set connection flags block/pause flag on all blocking commands by [@adiholden](https://togithub.com/adiholden) in [dragonflydb/dragonfly#2816 - chore: serialize SBF by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2846 - fix: test_replicaof_reject_on_load crash on stop by [@kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2818 - feat(dbslice): Add self-laundering iterator in `DbSlice` by [@chakaz](https://togithub.com/chakaz) in [dragonflydb/dragonfly#2815 - chore: License update by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2767 - fix(acl): incompatibilities with acl load by [@kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2867 - fix(json): make path optional in json.objkeys by [@kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2872 - fix: return wrong type errors for SET...GET command by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2874 - fix(redis replication): remove partial sync flow ,not supported yet by [@adiholden](https://togithub.com/adiholden) in [dragonflydb/dragonfly#2865 - chore: limit traffic logger only to the main interface by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2877 - chore: relax repltakeover constraints to only exclude write commands by [@kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2873 - chore(replayer): Roll back to go1.18 by [@dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2881 - fix: brpoplpush single shard to wake up blocked transactions by [@kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2875 - chore: LockTable tracks fingerprints of keys by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2839 - chore: reject TLS handshake when our listener is plain TCP by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2882 - Add support for Sparse HLL PFADD by [@azuredream](https://togithub.com/azuredream) in [dragonflydb/dragonfly#2761 - feat server: bring visibility to script errors by [@adiholden](https://togithub.com/adiholden) in [dragonflydb/dragonfly#2879 - chore: clean up REPLTAKEOVER flow by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2887 - chore(tiering): Move files and move kb literal to common by [@dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2868 - chore(interpreter): Support object replies by [@dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2885 - fix(ci/helm): Stick to v0.73.0 version of prom operator by [@Pothulapati](https://togithub.com/Pothulapati) in [dragonflydb/dragonfly#2893 - fix(acl): authentication with UDS socket by [@kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2895 - feat(cluster): add repeated ACK if an error is happened by [@BorysTheDev](https://togithub.com/BorysTheDev) in [dragonflydb/dragonfly#2892 - chore(blocking): Remove faulty DCHECK by [@dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2898 - chore: add a clear link on how to build dragonfly from source by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2884 - feat(server): Allow configuration of hashtag extraction by [@chakaz](https://togithub.com/chakaz) in [dragonflydb/dragonfly#2890 - fix: fix build under macos by [@BorysTheDev](https://togithub.com/BorysTheDev) in [dragonflydb/dragonfly#2901 - fix(cluster_replication): replicate redis cluster node bug fix by [@adiholden](https://togithub.com/adiholden) in [dragonflydb/dragonfly#2876 - fix(acl): skip http and add check on connection traversals by [@kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2883 - fix(zset): Better memory consumption calculation by [@chakaz](https://togithub.com/chakaz) in [dragonflydb/dragonfly#2900 - fix: fix ld for num converting by [@BorysTheDev](https://togithub.com/BorysTheDev) in [dragonflydb/dragonfly#2902 - chore: add help string for memory_fiberstack_vms_bytes by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2903 - fix(sanitizers): false positive fail on multi_test::Eval by [@kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2896 - chore: pull helio and add ipv6 replication test by [@dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2889 - chore: add ipv6 support for native linux release by [@romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2908 ##### Huge thanks to all the contributors! ❤️ **Full Changelog**: dragonflydb/dragonfly@v1.16.0...v1.17.0 </details>  Co-authored-by: repo-jeeves[bot] <106431701+repo-jeeves[bot]@users.noreply.github.com>

feat server: bring visibility to script errors

7b77bb7

Signed-off-by: adi_holden <adi@dragonflydb.io>

adiholden requested review from romange and dranikpg April 10, 2024 10:18

adiholden added 2 commits April 10, 2024 13:25

only under flag

1431fc4

Signed-off-by: adi_holden <adi@dragonflydb.io>

fix build

3207c57

Signed-off-by: adi_holden <adi@dragonflydb.io>

dranikpg previously approved these changes Apr 10, 2024

View reviewed changes

romange reviewed Apr 10, 2024

View reviewed changes

adiholden added 2 commits April 10, 2024 14:35

flip default flag value

7c85972

Signed-off-by: adi_holden <adi@dragonflydb.io>

expose error per sha in metrics

6fe1fda

Signed-off-by: adi_holden <adi@dragonflydb.io>

adiholden requested a review from romange April 10, 2024 19:38

dranikpg previously approved these changes Apr 10, 2024

View reviewed changes

romange reviewed Apr 11, 2024

View reviewed changes

add comment on map

9bb2ee0

Signed-off-by: adi_holden <adi@dragonflydb.io>

adiholden dismissed dranikpg’s stale review via 9bb2ee0 April 11, 2024 05:41

romange reviewed Apr 11, 2024

View reviewed changes

adiholden added 2 commits April 11, 2024 09:08

metrics expose only counter

30671fa

Signed-off-by: adi_holden <adi@dragonflydb.io>

remove include

27bfda0

Signed-off-by: adi_holden <adi@dragonflydb.io>

romange approved these changes Apr 11, 2024

View reviewed changes

adiholden merged commit 7580a88 into main Apr 11, 2024
10 checks passed

adiholden deleted the fix_2870 branch April 11, 2024 08:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat server: bring visibility to script errors #2879

feat server: bring visibility to script errors #2879

adiholden commented Apr 10, 2024

dranikpg left a comment

romange Apr 10, 2024

romange Apr 10, 2024

romange Apr 11, 2024

adiholden Apr 11, 2024

romange Apr 11, 2024

adiholden Apr 11, 2024

feat server: bring visibility to script errors #2879

feat server: bring visibility to script errors #2879

Conversation

adiholden commented Apr 10, 2024

dranikpg left a comment

Choose a reason for hiding this comment

romange Apr 10, 2024

Choose a reason for hiding this comment

romange Apr 10, 2024

Choose a reason for hiding this comment

romange Apr 11, 2024

Choose a reason for hiding this comment

adiholden Apr 11, 2024

Choose a reason for hiding this comment

romange Apr 11, 2024

Choose a reason for hiding this comment

adiholden Apr 11, 2024

Choose a reason for hiding this comment