Please help, I have created a Global variable monstrosity that destroys VMs

General
1

Hello nice Node-RED peoples,

I have done a bad. I know I have, but I don't know how to not.

So, in an effort to avoid having to learn how to database, I got the idea to just store every incoming MQTT message in a global array; just the current topic and payload. Updates when a new message comes in, overwrites the last payload value per topic.

These are light and switch states, so the payload is usually {"state": true} or {"state": false}, so the data ought to be small. I thought.

My rationale for doing this was that I could then query the current state in code elsewhere. For example, a person has entered the bog. Have they shut the door behind them? They have. Does the distance between the ceiling and the bog seat indicate they are probably sat down? It does. Serious business, then, best start the extractor fan

There I was, so convinced I had been very clever, waltzed away from the keyboard to go and do practical things in the world, and when I came back some hours later, the memory and disc space were maxed, and the spouse was unhappy because nothing was working. To be fair, I was unhappy too.

I run Node-RED in an LXC on Proxmox, with 4GB of RAM and 8GB of drive. I doubled both of these, and watched as they began to fill up over the next few hours. Clearly it's the new code, because when I disable that function, the problem goes away.

It seems, therefore, that I have fundamentally misunderstood how Node-RED manages memory. I don't seem to be updating the global array so much as copying it every time it updates, I think.

There are only 60ish topics, and the array is stable at that size - in other words, it's not just growing and growing.

Here is my stupid function. Please tell me what I am doing wrong. I would prefer to understand this in a way that prevents me from being this wrong again.

If this was a dumb idea from the get-go, I would like to understand why that is, too.

[{"id":"faa407de87fa8f70","type":"function","z":"c8268b7b6089484d","name":"Shove in a global array","func":"var obj = {                                 // Creates thing we're going to store.\n    \"topic\": msg.topic,\n    \"status\": msg.payload\n};\nvar states = global.get(\"states\") || [];    // If the global \"states\" array doesn't exist, return an empty one.\nvar big = states.length;                    // Is how we'll know if it's empty\nvar found = false;\nif (big == 0){                              // It was empty, so we shove a the new thing into it. Now it's not empty. Yay.\n    states.push(obj);\n} else {\n    let keys = Object.keys(states);         // Fine, it's not empty, so first let's check if we've seen this topic before\n    for (let i = 0; i < keys.length; i++) {\n        var temp = states[i];\n        if (obj.topic == temp.topic){       // Hello old friend.\n            found = true;                   // They were lost, but now they are found. But they're vegetarian, let the goat go.\n            temp.status = obj.status;       // Update the stored payload with the current one.\n            break;                          // annnnnd stop looking.\n        }\n    };\n    if(found == false){                     // If we still didn't find this topic, then its obviously unique, just shove the new thing in,\n        states.push(obj);\n    };\n};\nglobal.set(\"states\", states);               // This might be where the mistake is. Is this the mistake? I feel like it is.\nreturn msg;","outputs":1,"timeout":0,"noerr":0,"initialize":"","finalize":"","libs":[],"x":870,"y":480,"wires":[[]]},{"id":"5e03e7cd7cf7cdc0","type":"mqtt in","z":"c8268b7b6089484d","name":"","topic":"","qos":"2","datatype":"auto-detect","nl":false,"rap":true,"rh":0,"inputs":0,"x":570,"y":480,"wires":[["faa407de87fa8f70"]]}]

I thank you.

2

I'm not sure I understand everything in your function despite all the comments.
I think you need the context variable as an object not an array.

var states = global.get("states") || {}

Now you can either update or create an element with the same statement, so no chance of it running out of control.

states[msg.topic] = msg.payload

You can access a value with eg states.kitchen (I'm steering clear of your bathroom!)

Not only creepy but sexist too! :crazy_face:

1 Like
3

@jbudd 's suggest is sound as it is far simpler to create a global object.

If you really want an array then a function more like this is clearer

ìf(msg.topic && msg.payload){                  // msg.topic and payload exsists, you can also check they are the correct type etc
    const states = global.get("states") || [];    
    const index = states.findIndex(obj => obj.topic === msg.topic); //find index
    if(index >= 0){
        states[index].status = msg.payload;   // Update the stored payload with the current one.
    }else{                                   // If we still didn't find this topic, then its obviously unique, just shove the new thing in,
        states.push({                        // create new tyopic state
            topic: msg.topic, 
            status: msg.payload
        });
    };
    global.set("states", states);              
    return msg;
}
4

Hi @jbudd,
Thank you for your code suggestions.

None of the genders that live in my house stand up to take a dump. You do you, though, I'm not judging. :laughing:

For me, a toilet is a very good candidate for automation, especially if you have kids. I didn't even mention the little servo that squeezes a shot of airfreshener when user is done.

The goal, you see, is to have a house that reacts and predicts, such that I never have to touch a switch.

Anyway, your help is appreciated.

1 Like
5

Hi @E1cid,

Thank you for your code suggestion. If you have second, could you please clarify why you'd use const instead of var? Assume a strong level of ignorance on my part.

In any case, it turns out that the problem (probably) had nothing to do with the code.

The CMOS battery on the motherboard of the little machine I use as a server had died, resulting in no way to keep the real time. This meant that the system log began to grow with errors at a crazy rate, and filled the drive with a colossal log file which caused everything to explode.

I figured this out when time-based automations began to malfunction, and an attempt to recreate the node-red docker container was refused because of ... a time-based error, I forget what the actual message was.

Let me tell you, replacing a CMOS battery on an Intel NUC is no laughing matter.

Anyway, thank you for your help.

1 Like
6

let me google that for you, as i do not have a great deal of time this morning.

1 Like
7

:laughing:

Fair enough. Do you know what's gross about that website, though? It has a skin that looks like Google, but the results are all Bing.

For anyone that comes here via search, my current understanding is that const is for variables that shouldn't change or be re-declared. The elements inside a const can change, but the variable itself can't.

If I've got this right, that would make const a far better thing for a global variable, especially in the context I'm trying to use it.

I thank you.

8

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.