codewillclick / obparity

simple node.js library for creating objects that exist in tandem between client (browser) and server
MIT License
0 stars 0 forks source link

Manager object storage grows endlessly with every client-side refresh #13

Closed codewillclick closed 3 years ago

codewillclick commented 3 years ago

Need a check or way to know to remove dead objects.

Consider...

Do...

Fix:

Nicer, saner future fix:

codewillclick commented 3 years ago

Keepalives would just be... way too much traffic. It'd have to be class-level static function like that, which, on init, the ParityObject'd have to opt into. But then there'd be a need to configure the url for the keepalive, since that probably wouldn't be the same as any individual object's.

... Yeahhh, keepalive isn't looking likely, right now.

Actually I think this also covers "occasional report", so axing that one too.

codewillclick commented 3 years ago

Hmm, what kinds of different client/server pair relationships are there, anyway?

That makes two relationships that aren't one-to-one. But the many-server-to-one-client seems... unlikely. Unless multiple server nodes are active and a single client object is communicating with them all for redundancy's sake.

So let's do this.

Just forget about the one-client-to-many-server relationship. So that's 1c <-> 1s and Nc <-> 1s.

In the Nc <-> 1s, the server-side object is being used as a simple api endpoint. It adheres in a way to the original intent behind obparity, which is to bypass the whole api specification thing. As long as the client object knows which url to point to, you can interact with it like a normal object and not worry about the rest.

Though I guess most time is actually saved in having to write matching functions for both client and server objects, since the inherited method calls are just tossed up to the server to handle there.

Anyway, with many clients and one server, behavior is simple and need not be elaborated on. It's how this library was originally designed, after all.

So then we get to 1c <-> 1s. Here, it's executed with .pair() from the client side. So, unlike with Nc <-> 1s, the object exists on the client before it exists on the server. ... ... Is it just me, or does giving the client the power to spawn objects on the server side ad-infinitum sound like an absolutely horrible idea?

Should I begin with limits?

Should I make it look like it's spawning server-side objects, but really it's referencing a single server-side object with a bunch of extra attached data needed to maintain that illusion?

Well... no, I guess that isn't necessary. Right now, it doesn't make the server object second. It has to exist beforehand to be dumped into the paired bucket in the Manager. If one isn't already available, an Error is thrown. Okay, well... great. Almost got myself worried there.

codewillclick commented 3 years ago

So what're we looking at?

.createOnEmpty Manager config option. That's what creates a new object on the server side when there are no objects waiting to be matched with. I believe we just came to the conclusion that allowing this was a flawed precept.

1c <-> 1s is now being considered only when the objects are first deliberately created on the server side and therefore made available for the client to pick from.

I forsee issues where the client expects an object to exist, but some other client picked it up first. Hmm... Troubling. Well, that's a problem to kick down the road. For now, let's proceed with the idea that this problem has been mitigated, if nothing else.

codewillclick commented 3 years ago

But, like... we do still need to kill old objects.

codewillclick commented 3 years ago

Alright, the idea here is that a fixed number of objects are created every time someone, say... pulls down a specific asset with a page load or ajax or something, right?

Then, maybe a group object, a Nc <-> 1s situation, is created. Hmm... where the group/singleton/whatever object sits in the paired bucket but is still accessed when a new client object's pair() matches it. So no matter how many client sessions are created, they're still accessing the same object.

But then BOOM, a different user references the same asset. Now, a new group object needs to be created for them, even though there's one of the correct type already sitting there. Some kind of Session object would be like this.

The page load or login or whatever would prompt the creation of a server-side object, which maps by the new sessionId. But... how do we tell it to map by the session id, and not by the POST object's class id?

codewillclick commented 3 years ago

Hmm... hey why don't we allow it to spawn a new server object from the client side, but only allow it to spawn one object? Like, if we went by classId, but only created one object for every unique classId presented?

Well, the client could forcefully create new classId properties using the new modMessage() method. Then... it'd have to match an existing server-side classId, as well as be new to the Manager in order to approve the spawning of a new object.

It would force Nc <-> 1s behavior.

Would this defeat the idea behind parity objects? Hmm... In this case, it would be more like client-side API proxy objects with dynamically loaded source. Although, isn't that what it was from the beginning? (astronaut points gun, "Always has been")

It would also crush the Manager buckets idea like sparrows egg between thighs.

I'm thinking I'll have to undo that work, which is frustrating, because it took hours.

Then again, it may not have taken as many hours as it did to think my way through this problem.

I'll keep mulling over it.

codewillclick commented 3 years ago

Mulled.

I don't want to get rid of one-to-one objects.

I like the idea of a single client user session being represented by a pair of objects communicating exclusively with each other.

I don't want there to only exist API-representative objects on the server side.

Now, creating and using such API-ish objects is trivial, even right now; a url-match can map straight to any such singletons with zero issue whatsoever.

With .createOnEmpty disabled, the current code may be able to do exactly what I want, as-is.

Giving it a shot.

(But, you know... that still wouldn't solve the initial freaking problem! ... Though it might make timeout removal somewhat feasible.)

codewillclick commented 3 years ago

Timeout removal would be easy, but its working properly would not be provable. Not without keepalives, which I've already shelved.

That leaves... some kind of session id dependence. I'll have to think about this... even harder.

codewillclick commented 3 years ago

"There are two keys fighting inside you. One is key A. And one is key B."

"Which one wins?"

"Not you. You explode from the inside. Never start a fight inside your literal body my friend."

...

More to the point, key A and key B are references to properties a parity message object passes up. Or maybe something in the post body itself. So I guess it's meant to be used in conjunction with modMessage()?

Maybe if provided a function, it'd take the message object as a parameter?

One is a session key, an on-change key. The other is the id by which to find all the server-side objects that still match the old on-change key and eradicate them.

So there'd need to be a table matching key A and key B to all the server-side objects being managed.

Where the premise is that there can only be one active keyA value at a time, and any object associating with an old key A value is invalid and to be collected for disposal.

codewillclick commented 3 years ago

Key B should be something like an identifier for the ParityObject. Since it should only matter for objects that're already paired, both client and server objects should have the same parityId. This could work as a default key B. The onChange key, then, would be where the logic sits.

A table to keep track of all objects using a specific session id value.

Wait a second, let's think there... keyB is going to be changing all the time. We can't count on matching between keyA and keyB. We... really might only need keyA.

Yeah, so if the first time keyA changes for anything, all objects referencing keyA are deleted, then that leaves no room for keyB.

But what about a pairing unrelated to keyA? Completely different session type? Maybe keyB should be something like a session... tag? A session type? Otherwise, how do you know to refer to a specific keyA?

Oh no, I'm going to have to switch around the meanings of key A and B in my head. Nuts.

codewillclick commented 3 years ago

keyA:

keyB:

When calling pair(), those values should be provided.

pair(sessionKey, sessionId) { ... }

sessionKey is keyA. sessionId is keyB. I was thinking keyB could be passed in as a function, but that won't fly, since it's being stored as JSON in the message object.

Maybe then a getChangeKeyOnPair() method? But what would call it? And would overriding it require overriding modMessage() as well?

ParityObjects are matched normally by .classId in Manager's internals. And pair() only has room for special logic through Manager's evaluate() method.

Well, I don't like having to pass two params into pair() every time, but if that's the only reason I'm hesitating, having found a workable solution to this problem, then I need to rethink my priorities.

Yeah it doesn't make things simpler, looking at it in use, but it does solve a real problem.

I could also look into splitting pair() into two different methods. One to pair with an API object (the Nc <-> 1s case), and one to pair with a server-side object under the premise of mutual exclusivity (the 1c <-> 1s case).

Hmm, well, point being, I think I've thought my way to a plan of action.

codewillclick commented 3 years ago

pair(ob): api pairing pair(key,id): one-to-one pairing

It doesn't work that way right now, but... well let's get the second argument permutation done for now. That'll be my plan of action, and that'll close out this freaking issue. Which I've been stuck on for days now.

codewillclick commented 3 years ago

Hmm, maybe in the future consider making Manager buckets. Where a bucket is made that objects are dumped into, and whenever a new session is made, and its new objects created therein, the old bucket as a whole is just discarded.

It would be like... Manager.createBucket(sessionKey, template), and template here would be the lump sum of all pobs that are going to be paired with. So every application will have a bunch of Manager bucket templates for their various needs.

But for now, it's enough just to have that one sessions table.

codewillclick commented 3 years ago

I'm running into an issue where the more I make all this work, the nastier the code looks.

I'm really going to have to clean this mess up. Ew.

Oh well, the name of the current Milestone is "Ease of Use", not "Ease of Comprehension".

Abidance to any level of sanity can wait for the next major milestone.

(Author deigns to ignore the fact that "use" includes use by those intending to modify or extend, so...)

codewillclick commented 3 years ago

It WOOORRRKS!!!

Woohooooo!

Aw yeeeyaah!

...

...

Except...

Except that...

Sometimes it doesn't.

codewillclick commented 3 years ago

And that appears to be when the page refresh happens so fast that new ParityObjects are added to this.unpaired before they have a chance to actually receive their respective pair()s from the client.

Hmm... the first new pairing with the same sessionKey but different sessionId should wipe out all the old ones, as it seems to do with this.paired, but it looks like it leaves this.unpaired hanging.

It think it's that this.unpaired has no hooks into the sessionId values.

And those would be added in... here in Method's accept().

accept(pob) {
  // In case the pob wasn't created with the Manager, add it in like this.
  let stack = this.unpaired[pob.constructor.sourceClassId]
  if (!stack)
    stack = this.unpaired[pob.constructor.sourceClassId] = []
  // Push it into the appropriate stack and... I guess that's it.
  stack.push(pob)
  // ^ HERE, right HERE
  return this
}

That's where something needs to be added so that the old-session-object deletion process will know what to remove.

Come to think of it, only one point-of-entry for this.unpaired, huh? That's good design practice! Good job, me!

codewillclick commented 3 years ago

Come to think of it, only one point-of-entry for this.unpaired, huh? That's good design practice! Good job, me!

Why, thank you!

codewillclick commented 3 years ago

Oh! Ohhhhh! And that horrible this.unpairedRefs hack actually works! Holy nuts, I can finally close this issue! Good riddance!

And good luck future me when refactoring time comes... Yer gonna need it.

codewillclick commented 3 years ago

And good luck future me when refactoring time comes... Yer gonna need it.

... ... ... Thanks.

codewillclick commented 3 years ago

Ahh, right. Forgot to mention. The reason this hack worked is because I added a few params to Manager's accept() and create() methods. They look like this, now:

accept(pob,sessionKey,sessionId)
create(pob,match,sessionKey,sessionId)

See that? Now we can declare their session key and id ahead of time. This takes care of the problem of pobs waiting for exclusive pairing not knowing which session they belong to.

Well... it's hacky as nuts, compared to what I had in mind.

It's the opposite of Ease of Use, too.

But I'm keeping it, because it (mostly) solves this issue in a provable manner.

I'll throw in a timeout, later, just to get rid of any last hanger-alongs.