Do not always hide atoms when using frames.

[linas]

When multiple frames are being used, the current rocks code will just hide atoms, instead of removing them, when frames are being used. This results in functionally correct behavior, but is wasteful of storage if the atom can actually be deleted.

The AtomSpace extract code currently implements the correct (reference) implementation: it will either hide an atom, when needed, or it will delete the atom, if possible. It can be used as the final arbiter of whether to delete, or not. The backend should follow this advice.

There right way to solve this is to implement the pre-delete and a post-delete calls in the backend. The atomspace calls pre-delete before doing the deletion, and rocks can gather any needed info for the deletion to happen. Next, the atomspace extracts the atom. Then it calls the post-delete hook. The hook code should look to see what the atomspace did: either the absent flag is set on the atom, or the atom is actually gone. If its only marked absent, then rocks should also hide the atom. else, rocks should delete the atom.

[sikefield3]

I'm interested in doing this.

When multiple frames are being used, the current rocks code will just hide atoms, instead of removing them, when frames are being used. This results in functionally correct behavior, but is wasteful of storage if the atom can actually be deleted.

What I found is that currently, the actual hiding takes place with _rfile->Put(...) in RocksStorage::storeMissingAtom?

The AtomSpace extract code currently implements the correct (reference) implementation: it will either hide an atom, when needed, or it will delete the atom, if possible. It can be used as the final arbiter of whether to delete, or not. The backend should follow this advice.

There are two ways to solve this:

1. implement rocks so that it tries to use the same decision logic as the atomspace, when removing. This risks having the two be subtly out of sync.

2. Have a pre-delete and a post-delete call in the backend, so that first, pre-delete is called (and rocks can gather any needed info for the deletion to happen) then the atom is extracted from the atomspace, and then the post-delete code looks to see what the atomspace did: either the absent flag is set on the atom, or the atom is actually gone. If its only marked absent, then rocks should also hide the atom. else, rocks should delete the atom.

Seems that 2 is a better solution.

This is my suggestion how this could be accomplished:
The deletion of the atom in the AS has already happened (I'm not sure about this.). In the recursive function RocksStorage::removeAtom and if we have multi-space, then we can do the following steps:

1. look into the atomspace if the absent flag is set.Handle lookupHandle(const Handle& h) const or bool Atom::isAbsent() const resp. seem to be the functions to get the absent flag ?
2. Either we delete with _rfile->Delete or do the same as in RocksStorage::storeMissingAtom. I assumed that it is enough to check for the absent flag in every recursion step, if not one has to do the whole recursion twice ?

Anyway, I hope I didn't get this entirely wrong! Maybe you could point out what i´m missing.

[linas]

Wow! Cool! Umm. Technical questions require detailed answers;

the actual hiding takes place with _rfile->Put(...) in RocksStorage::storeMissingAtom?

Yes I think so. I wish there was some documentation in the code that explained what that function did! What moron doesn't document their code??? Oh, wait ...

Here's a trick: RocksStorage::print_range prints a range of records in Rocks, in the "raw" format, as strings. You can use it to see what is actually being stored. You can use it from scheme like so:

(cog-rocks-get "a@")  ;;; print everything starting with "a@"
(define rsn (RocksStorageNode "rocks://... whatever"))
(cog-rocks-print rsn  "a@")  ; same as above

Use empty string to dump everything. If you're stepping through examples/demos, you can use this to look at the actual contents of the DB, to see if it matches up with the docs, with what you think it should be.

The deletion of the atom in the AS has already happened (I'm not sure about this.).

Well, that's the main issue. If I recall correctly, what needs to happen is that the backend needs to be warned that the deletion is going to happen; then the atomspace can delete, and then the backed can be told to finish up with whatever it needs to do. Without this two-step, there's just not enough info available to "do it right": either the atom is already gone and we've lost access to it, or its not yet gone, and we don't know what to do with it next. A kind-of chicken-and-egg problem.

Anyway, I hope I didn't get this entirely wrong!

I don't know. Maybe the most important thing is to go through the examples & demos and really be sure that you've got a clear idea of how its supposed to work. I think the code is bug-free (famous last words). The design goal was to make it all "simple and obvious" for the user, which makes the implementation rather complicated. (I recall that it was hard to get it right.) If you're going through the examples, and something seems wrong ... well, that could be a bug. If you're not sure, the cog-rocks-print debug utility is great for exploring what's actually in the DB.

[linas]

Oh, several side remarks:

• The CogStorageNode, which implements storage over the network, has almost no support at all for frames. Only half of one unit test works. The rest is a blank slate. It would be nice to get this working, but ... it would be a huge and difficult effort. It's likely to expose other chicken-n-egg issues.

• There are other performance issues. One of my favorites is to call (cog-report-counts) to print a summary report of what is in the AtomSpace. It's fast and works great in the base AtomSpace; its painfully slow when its in a higher layer. I don't know why. (my datasets contain tens of millions of atoms these days. So "painfully slow" is anything that doesn't clear 100K atom operations/second.)

• Most of the AtomSpace has been very highly tuned for performance. So almost all the juice has been squeezed out. There is only one weird trick left, that might get some more out, but it would be complicated and difficult. But this is off-topic.

[sikefield3]

Tried to test this in some - as it seemed to me - suitable example with two AS + RocksDB to see if the atom gets deleted.

In the situation of this example, we have a base + overlay AS. Now, one can attach a Rocks DB to the overlay.

(define base (cog-atomspace))
(define ovly (cog-new-atomspace base))
(cog-set-atomspace! ovly)
(define rsn (RocksStorageNode "rocks:///home/opcwdir/repodock/ForTestingStuff/foo.rdb"))
(cog-open rsn)

Now, add sth to the base and check if it is also in the overlay, then add it to rocks:

(cog-set-atomspace! base)
(List (Concept "A") (Concept "B"))
(cog-set-atomspace! ovly)
(cog-prt-atomspace)
(store-atomspace )
(cog-rocks-print rsn  "")

Then go back to base and delete some atom:

(cog-set-atomspace! base)
(cog-delete-recursive! (Concept "B"))

Now, if you look in both overlay and rocks, the deletion did occur as expected.

[sikefield3]

Later, I want to try other examples with multiple atomspaces (e.g. frame)

[linas]

Then go back to base and delete some atom:
...
the deletion did occur as expected.

It is not obvious if this is the desired outcome, or if it should even be allowed. If you think of overlays as going from past to future, i.e. the base space containing the oldest data, and the overlays as being newer, then deleting something in the base space is like changing something in the past. Should that be allowed, or not? What might a reasonable person expect to happen? (I don't yet have good answers for any of this. I kind of follow my nose, to see what "feels right".)

Some examples:

• Create (Concept "B") in the base. Then go to the overlay, and create (List (Concept "A") (Concept "B")) Now, go to the base, and delete (Concept "B") ... what should happen to the stuff in the overlay? Should it still be there? Should it be gone? Should there be a warning message "I'm sorry, Dave, I can't allow you to do that"? Should the overlay be protected from damage to the space underneath? Off the top of my head, I don't know.
• Create (Concept "B") in the base. Go to the overlay, and set some value on it (say, a truth value). Now, go to the base, and delete (Concept "B") ... what happens to the stuff in the overlay? Should it still be there with the modified value, or should it be gone?

Currently, I have only two use cases:

• The overlay is used to hold temporary, partial results, and then it is destroyed at the end. The base space holds the "permanent" results. (this is what the query subsystem does).
• There is a stack of frames, recording a history of changes. Each new layer contains deltas on the layer beneath. Lower layers, the "history", "the past", are never changed. (Some of my learning code does this).

Anything outside these two use cases is unexplored. My gut instinct says that it would be best if lower layers cannot be modified. This includes deletion. There is, however, a flag that sort of allows this; it is used to modify the base while using a temporary (because some results obtained in the temporary need to be copied back into the base.)

[sikefield3]

As a starting point for an example with subframes, I looked at the multi space example and I tried to see what happens in the rocks database, if I delete an atom:

(cog-set-atomspace! a)
(Concept "A")

Attach Rocks storage to C:

(cog-set-atomspace! c)
(define rsn (RocksStorageNode "rocks:///home/opcwdir/repodock/ForTestingStuff/foo.rdb"))
(cog-open rsn)
(store-atomspace)
(store-frames a)
(store-frames b)

Now we delete the atom from the subframe A:

(cog-delete-recursive! (Concept "A"))

The atom disappears from C (as we can check with (cog-prt-atomspace), but not from the RocksDB (cog-rocks-print rsn "a@") .

Note: Now, we have the problem that atom is still in A, which might not be intended ?

[sikefield3]

Now, let's try a different (maybe more plausible) scenario (again from the multi-space example):
delete an atom in atomspace A:
We are in the following situation:

(define a (AtomSpace))
(define b (AtomSpace))
(define c (AtomSpace a b))
(cog-set-atomspace! a)
(Concept "I'm in A")

and rocksdb links to atomspace c:

(cog-set-atomspace! c)
(define rsn (RocksStorageNode "rocks:///home/opcwdir/repodock/ForTestingStuff/foo.rdb"))
(cog-open rsn)
(store-atomspace)
(store-frames a)
(store-frames b)

Now, "I'm in A" is deleted from atomspace A (in contrast to the previous example, where the atom was deleted from C) :

(cog-set-atomspace! a)
(cog-delete-recursive! (Concept "I'm in A"))

and again it is purged from C, but not from rocksdb.

[sikefield3]

Now, let's try a different (maybe more plausible) scenario (again from the multi-space example):

Another modification from the previous example I just tried: have exactly the same atom (Concept "AB") in atomspaces A and B (is this likely to happen?).
What happens, if we delete this atom? As expected, if we want to erase (Concept "AB") from C, it is necessary to apply cog-delete-recursive! to both A and B.
But as above, the atom still remains in the rocksdb.

[linas]

This was mouldering in my email box. Sorry for the late reply. Regarding this comment: #15 (comment)

Working as designed. The Atom is not deleted, it is hidden. Let me explain. You wrote:

Now we delete the atom from the subframe A:

(cog-delete-recursive! (Concept "A"))

The atom disappears from C (as we can check with (cog-prt-atomspace), but not from the RocksDB (cog-rocks-print rsn "a@") .

Try (cog-rocks-print rsn "") and you will see:

rkey: >>a@3:<<    rval: >>(ConceptNode "A")<<
rkey: >>f@(as "(uuid . 2)")<<    rval: >>4<<
rkey: >>f@(as "(uuid . 3)")<<    rval: >>5<<
rkey: >>f@(as "(uuid . 4)" 4 5)<<    rval: >>6<<
rkey: >>k@3:4:+1<<    rval: >><<
rkey: >>k@3:6:-1<<    rval: >><<

This states that:

• There is an Atom (ConceptNode "A") and it is assigned a UUID of 3
• There are three atomspaces with (confusing, off-by-two) UUID's 4,5,6. These are the f@ (The off-by-n effect is purely due to order of creation. Try e.g. adding more atoms, before creating more atomspaces.)
• Number 4 is AtomSpace "a" and number 6 is AtomSpace "c"
• (ConceptNode "A") is alive and well in 4: that is what >>k@3:4:+1<< says.
• (ConceptNode "A") is hidden in 6: that is what >>k@3:6:-1<< says.

What happened when you did the delete, you only deleted in frame c, but not in frame a. When you say delete-recursive the "recursive" does not refer to frames, but only to containing links. (So for example, if you had (Link (Concept "a")) then a regular delete on (Concept "a") will error out, because its contained in the link. A delete-recursive will delete the link, without complaining.)

If you closed the storage node, exited, re-opened, restored, you would find that (Concept "a") is present in a, but absent in c (i.e. just as before). The idea is that one can put a bunch of stuff into the base space a, and any changes in c are just a "delta" on a. In particular, deleting from c must not delete from a.

The k@ -1 and +1 are "hide" and "expose" markers. You can continue layering more and more atomspaces, alternately creating and deleting (Concept "a") in each, and you will see these k@ hiding and exposing in each layer.

[linas]

Regardng this comment: #15 (comment)

I see an actual bug. But before I explain the bug, let me point out a variation on what you wrote, that does work correctly. If I take what you wrote and also save c, then everything is OK. (cog-rocks-print rsn "") shows that a@ for (ConceptNode "I'm in A") is there. It will always be there, forever and ever. The most that can happen is that it would be hidden with a k@ And, for this example, if I look at the k@ I see that it is indeed hidden in atomspace a. it is unmarked in atomspace b,c and therefor absent in those.

The optimization, proposed at the very top, is to just go ahead and remove the a@ since it is not visible in any frame, ever.

Along the way, I seem to have spotted an actual, real-life bug. Next comment.

[linas]

I opened issue #21 to describe the bug.

[sikefield3]

After some lazy weeks, here some suggestions...:
Looking at the code in AtomSpace::extract_atom, there are two ways an atom can be deleted in the atomspace:

• hide with Atom::setAbsent(void), this can be checked.
• bool TypeIndex::removeAtom(const Handle& h): this looks like a delete from memory function.

Sometimes, atoms from the incoming set also get deleted.

To handle deletion in RocksDB, one could do this:

• Create one unique SID (session id) for the whole process
• Deleting from RocksDB: If an atom is going to be hidden (in RocksStorage::storeMissingAtom ), memorize it in some place TEMP for later:
  In RocksDB, there could be put some kind of index (or special place) , that points to all the entries that are hidden (with some unique id, so everything can be found quickly). Or attach the id to the "k@" entry and later search for it (seems slower).
  Alternatively, one could memorize in RAM or in some special atomspace (if this is a thing?).
  I think we have to store the id of the atomspace as well.
• Delete from atomspace
• post-delete: go over all entries in TEMP: retrieve the corresponding atom in the atomspace and look, if it is hidden or deleted (Atom::setAbsent). If it is deleted, then delete it ("k@" + "a@" entries) from RocksDB. Also clear TEMP

To elaborate further:
We could store the temporary entries for the atoms that are to be deleted in ´TEMP´ like this:
"t@"+SID+sequential number, so they can be in a contiguous space in RocksDB.

[linas]

Some comments, then:

Rocks is a single-user system. There shouldn't be a need for a "session id" -- there can only ever be one session, ever.

So let me expand on suggestion 2) up at the top. The design would be:

• class BackingStore should get two new methods: pre-delete and post-delete Maybe something like this:

virtual void preRemoveAtom(AtomSpace*, const Handle&) {};
virtual void postRemoveAtom(AtomSpace*, const Handle&) {};

• Modify the AtomSpace extract code to call these two. But only call them if the atom is really going to be deleted, and not just hidden. There's logic already in the atomspace delete code that can tell these two cases apart; the callbacks need to be plugged into the right places.
• Implement the two callbacks above, such that the second one actually does delete. The postRemove callback checks to see if the absent flag is set. If it is not set, the atom can be removed.

[sikefield3]

It looks like we have to pass the class BackingStore to the AtomSpace::extract_atom function?
If we do this, we could simply delete the database entries from the atomspace code ? We step over the atoms of the incoming set and call (a modified version of) RocksStorage::storeMissingAtom which does the deletion form the DB. So, we have only one loop over the incoming set.

In more detail - it could be done in the following way:

1. in StorageNode::remove_atom check if we have a multi-space
2. if yes, don't go into the remove function of RocksDB
3. instead call AtomSpace::extract_atom with RocksDB as an extra argument, run over the incoming set and if we delete an atom from the atomspace, we give the handle to storage node + tell it if it should hide or delete.

[linas]

Hi @sikefield3 sorry I lost track of my email.

Let me review the overall structure, starting from the user API. By this I mean "legal calls the user can make into scheme or python or C++". They can do one of two things:

• delete an atom from the local AtomSpace, only,
• delete an Atom from the local AtomSpace, and also the attached storage.

The first is done by calling cog-extract! and the second by cog-delete! (These are the two scheme funcs; there are analogous python funcs). The first calls SchemeSmob::ss_extract and if you look at line 763 you see it calls AtomSpace::extract_atom() and that's it. Nothing more to be done. There's no backing store, there's no storage. Nothing more needs to happen.

The second case is more complicated, and its the one that needs the update. Here's the call chain:

• cog-delete! calls PersistSCM::sn_delete which calls StorageNode::remove_atom() at line 331
• The StorageNode::remove_atom() is here and at line 127 you can see it call BackingStore::removeAtom followed by a call to AtomSpace::extract_atom at line 132. Recall that BackingStore::removeAtom is a pure-virtual method, and each different storage node provides some custom implementation.

It's this last part, StorageNode::remove_atom() that needs to change. The pseudocode needs to be this:

StorageNode::remove_atom() {
    if (read-only) as->extract_atom(h, recursive);
    else {
        preRemoveAtom(as, h, recursive);
        as->extract_atom(h, recursive);
        postRemoveAtom(as, h, recursive);
    }
}

That's all that is needed on the core AtomSpace side. This will allow the comment about WriteThruProxy to be removed, because the two-step pre-post remove will be able to fix things just so.

For rocks, one just has to implement a RocksStorageNode::preRemoveAtom that is nearly identical to the current RocksStorageNode::removeAtom and also add a new RocksStorageNode::postRemoveAtom that will do whatever else remains to be done.

Does that make sense? See what's happening?

[linas]

I've implemented the code for the comment immediately above in opencog/atomspace#3044 I think this will simplify your thinking about all the other parts of this issue.

[sikefield3]

Cool!

[sikefield3]

Somehow, I wasn't aware of the whole class hierarchy:
BackingStore -> StorageNode -> ProxyNode -> WriteThruProxy
Same for ReadThruProxy
(Also StorageNode has another ancestor, Node)

For RocksStorage, there is
BackingStore -> StorageNode -> RocksStorage
This should do for now...

[linas]

Yes! Here's the design:

• BackingStore is a bunch of callbacks. The AtomSpace calls them when needed, to tell storage to do things. Things like Rocks have to implement the callbacks. The callbacks are "private", in that only the AtomSpace is allowed to call them.
• StorageNode is an Atom (a Node) that wraps up BackingStore. Since it's an Atom, you can put it in the AtomSpace. It provides a "public" API that users can call, to work with storage. This include open, close, store, fetch, etc.
• RocksStorageNode is one kind. There are others: one old moldy one for Postgres, one for network, one limited one for flat files.

Recall, its all in the wiki: https://wiki.opencog.org/w/StorageNode

The ProxyNode is the cool new thing, for me. I needed to solve some old problems:

• Evict old, unused Atoms from the AtomSpace (to free up RAM) but save them to disk. (The old "attention bank" used to do this, but it sucked.)
• Send reads and writes to several network nodes, not just one. Route them. (Our patron saint Ben Goertzel has been infatuated with a "distributed AtomSpace" that runs over large swath of the network. The ProxyNode provides a mechanism to do that.)
• Caching: go to storage, only if we don't have a recent copy of an Atom (because storage might be far away and slow, so touch it only if needed.)
• Mirroring: yoke together several storage, treat them as one. (For example, one storage might contain only genes; a second only proteins; a third is the users scratch-space. The first two are read-only and huge, the third is read-write and is a dumping ground for whatever data the user generates.)

So, different ProxyNodes can do all these. They keep the old StorageNode API, so nothing changes, its "transparent" from the users point of view. Since everything is an Atom, the wiring diagram of what's connected to what is stored in the AtomSpace.

I implemented the simplest ProxyNodes as "proof of concept"; I have not done the fancy ones yet. Mirroring is easy. The Eviction one has not been implemented, it's probably the most important/interesting one to do next. Some of my data runs are just a bit too big to fit into RAM, so I have to kill them and restart. It would be nice to have some EvictionProxyNode where I can tell it "don't use more than 64GB or RAM" or something like that. I might do that ... someday. There's some tricky parts in understanding what "the right thing to do" is. I'm desperately trying to avoid storing LRU timestamps...

[sikefield3]

Here is how preRemoveAtom and postRemoveAtom could be implemented in RocksStorage:

• RocksStorage::preRemoveAtom(as, h, recursive);: This is a somewhat changed version of removeAtom:
  Set the entry in the RocksDB temporarily to "-2" instead of "-1".
• as->extract_atom(h, recursive);: No change needed here
• RocksStorage::postRemoveAtom(as, h, recursive);: We want to find out what has been done in the atomspace.
  In RocksDB, go over all DB entries of the form "k@" marked with "-2": find the corresponding "a@sid" -entry,
  use the sid in RocksStorage::getAtom(const std::string& sid) to find the atom in the atomspace.
  Call Atom::isAbsent() to look into the atomspace, if the atom is deleted or hidden. If it is hidden, set DB entry to "-1",
  otherwise, delete it from RocksDB with RocksStorage::doRemoveAtom.

PS: About the RocksStorage::getAtom: Somewhere Handle Sexpr::decode_frame is called (see definition), hence the correct frame should be found?

[sikefield3]

While I was trying to figure out, why the tests with my implementation of the RocksStorage::postRemoveAtom function fails, I stumbled upon this:
If you write an atom into Rocks, usually, you get a k@ record with a deletion mark "+1". But if you have an atom with a truth value, you have only "1" as shown here:

rkey: >>k@5:2:+1<<    rval: >><<
rkey: >>k@6:2:1<<    rval: >>(CountTruthValue 1 0 5)<<

Is there a particular reason for this ?
This has just caught my eye, when I was running the tests with my changes and several of them that use truth values(like this one) failed.

[linas]

Hi @sikefield3 My apologies for a very late response. I am seeing your work just now. For the question about +, the answer lies here:

atomspace-rocks/opencog/persist/rocks/RocksIO.cc

Lines 79 to 107 in 08b21ed

	// Common abbreviations:
	// ---------------------
	// satom == string s-expression for an Atom.
	// sval == string s-expression for a Value.
	// senc == string s-expression for an AtomSpace (Frame).
	// stype == string name of Atomese Type. e.g. "ConceptNode".
	// aid == uint-64 ID. Every Atom gets one.
	// sid == aid as ASCII string.
	// kid == sid for an Atomese key (keys must always be Atoms)
	// fid == sid for an AtomSpace frame.
	// skid == sid:kid pair of id's
	// shash == 64-bit hash of the Atom (as provided by Atom::get_hash())
	//
	// Prefixes and associative pairs in the Rocks DB are:
	// "a@" sid: . [shash]satom -- finds the satom associated with sid
	// "l@" satom . sid -- finds the sid associated with the Link
	// "n@" satom . sid -- finds the sid associated with the Node
	// "k@" sid:kid . sval -- find the Atomese Value for the Atom,Key
	// "i@" sid:stype-sid . (null) -- finds IncomingSet of sid
	// "h@" shash . sid-list -- finds all sids having a given hash
	//
	// Multi-AtomSpaces also use the following keys:
	// "d@" fid . senc -- finds the AtomSpace frame (delta) for fid
	// "f@" senc . fid -- finds the fid associated with the AtomSpace
	// "k@" sid:fid:kid . sval -- find the Value for the Atom,AtomSpace,Key
	// Absent Atoms have a kid = -
	// Keyless Atoms have a kid = +
	// "o@" fid:sid . (null) -- find Atoms in a given frame
	// "z" N@sid . (null) -- record height N of Link at sid

and I specifically see this:

// "k@" sid:fid:kid . sval -- find the Value for the Atom,AtomSpace,Key
//                            Absent Atoms have a kid = -
//                            Keyless Atoms have a kid = +

and so it would seem three possibilities: a number, a +number and a -number. Looking at the code, it seems like only +1 and -1 appear, for example, near lines 329 383 412 which seem to say that +1 is a temporary marker. The important one seems to be near line 638 which says

      // If there is just a + instead of a key, this means that
      // the atom is in this frame, but has no keys on it. Insert
      // into frame, and return. There can never be more than one
      // of these per frame, so we return immediately.

So it appears that + indicates an Atom with nothing attached to it. In other parts of the code, it would see that I used a +1 instead of a naked + and a -1 instead of a naked -. I don't recall why I used +1 and -1 instead of plain + and -.

About your idea of using -2 I would suggest instead to use one of the other symbols #$%^&=!|? for this. This allows a test of a single-byte compare, instead of a string-compare.

[linas]

BTW, I plan to make unrelated changes in the next few hours. You should take your existing work, and make a pull request, and then say something like "pull request is a draft for review" then I won't merge it, but at least I can explore what's going on in there.

[linas]

For your earlier comment:

Set the entry in the RocksDB temporarily to "-2" instead of "-1".
[...]
In RocksDB, go over all DB entries of the form "k@" marked with "-2": find the

My gut instinct is that you don't need to write anything out during this process. For any given thread, you are guaranteed that if there's a call to preRemove() then there will be a call to postRemove() for exactly the same atom, and that nothing else will intervene in the middle, and so you don't need to write anything to disk. You'll never get these out of order.

There is some slim chance that some other thread will try to remove the same atom, so that there is a preRemove() in thread 1, then thread 2, then postRemove() in thread 1 then thread 2, and this interleaving does create opportunities to bungle things. But making this work right usually is not hard, and usually does not require locks. If you'r uncertain, I can explore and fix any such race conditions. One of the unit tests does this: it adds and removes the same small set of atoms, over and over again, as fast as possible, in a bunch of threads. If something croaks, it will be that one.

[sikefield3]

About your idea of using -2 I would suggest instead to use one of the other symbols #$%^&=!|? for this. This allows a test of a single-byte compare, instead of a string-compare.

Sure, I can do that.

[linas]

Oh hey, you're there! My apologies for not answering in so long! (I screwed up.)

I'm not sure if I should interact with you a lot, or if I should take a "hands-off" approach. If you're unsure of an implementation idea, and want to talk about it, make a pull request, but set it into "draft" form. That makes it a lot easier to review & discuss. Of course, it takes brain-cells & real thinking on my part to review what you're doing, so I'm happy to go hands-off. But if you're spinning and floating and a little lost, its much better if you continue to ask. And if I don't respond in a few days, then ping me one, two, three, four times. I'm sometimes distracted and need the pings to re-prioritize my to-do list.

[sikefield3]

Oh hey, you're there! My apologies for not answering in so long! (I screwed up.)

No problem! I was preoccupied with other things as well, so there didn't anything happening in the last weeks.

I'm not sure if I should interact with you a lot, or if I should take a "hands-off" approach. If you're unsure of an implementation idea, and want to talk about it, make a pull request, but set it into "draft" form. That makes it a lot easier to review & discuss. Of course, it takes brain-cells & real thinking on my part to review what you're doing, so I'm happy to go hands-off. But if you're spinning and floating and a little lost, its much better if you continue to ask. And if I don't respond in a few days, then ping me one, two, three, four times. I'm sometimes distracted and need the pings to re-prioritize my to-do list.

I think the approach with draft pull requests is a very good idea. This way, you can see the misconceptions I still have. In fact I was going to make a PR shortly.

About my approach: I am still figuring things out, while coming up with the code. I also use the examples (by modifying them etc.) to see what's going on. In addition, I tried to use gdb to step through some parts of the code, but that proved just a little to cumbersome...

And don't worry, if it takes you a bit longer to respond.

[sikefield3]

Just made this Draft pull request

[sikefield3]

For your earlier comment:

Set the entry in the RocksDB temporarily to "-2" instead of "-1".
[...]
In RocksDB, go over all DB entries of the form "k@" marked with "-2": find the

My gut instinct is that you don't need to write anything out during this process. For any given thread, you are guaranteed that if there's a call to preRemove() then there will be a call to postRemove() for exactly the same atom, and that nothing else will intervene in the middle, and so you don't need to write anything to disk. You'll never get these out of order.

Sorry for the late response:
I didn't think about multi-threading at all. My basic assumption is, if you want to delete an atom from RocksDb, you have to keep track of a bunch of entries in one way or other, as explained below:
In preremove: just find the atoms in RocksDb that should be considered for deletion, but we can't delete just now, so we mark with "-2" or whatever (because there are old entries with "-1" and we don't want to worry about them).
Then do the deletion in the atomspace.
Finally, go over RocksDB again: Now that we know what happened in the atomspace, we can either delete completely, or mark with "-1".
The purpose "-2" marker is to temporarily keep track of the records we might want to delete.
As Another way of keeping track, it might be preferable to do nothing in RocksDB in preremove, but instead keep a local list of record keys ? ... Yeah, that might make a huge difference in speed .... ?

Edit: just had clean sth up

[linas]

Let's go back to square one. Restate what the problem actually is, and what the solution needs to do. I should preface: the code works perfectly today and this project is about creating a performance optimization for a special case that involves frames.

Please note: almost no one uses frames! There's no screaming, burning need to actually make atom deletion work better: it already works just fine. The only "problem" is that it could use slightly less disk space, for the special case of atoms being deleted in frames. By "slightly less", I mean maybe 100 bytes per atom, and so a user would have to be using frames (and most users don't) and would also have to have 10 million deleted atoms, before they'd notice a (mere) 1GB extra disk-space being used. And they probably won't notice because if their data set is 10GB, and for some reason its 11GB because the delete function wasn't optimized, who cares?

So this project is a pretty damn obscure performance optimization inside the rocks code base. It's a reasonable way of learning how the atomspace works, and dinking with it, but it is not a high-priority project, and I'm thinking that perhaps there are much more important and better projects for you to work on, @sikefield3 -- stuff that I actually kind of care about and will have impact.

Heh. OK, with that out of the way, let me go back and describe the actual problem.

If the user is NOT using frames, then delete works fine, and nothing needs to be done.

If the user is using frames, then there is the following scenario:

• User creates atom in the base space
• User pushes a frame (All atoms in the base frame are visible, but now become "read-only")
• User wishes to delete some atom (that exists in the base space). Since the base space is read-only, it cannot be actually deleted. Instead, it is marked "hidden". To the user, it looks as if it's deleted; its not, its still there in the base, its just marked invisible in the higher layer. This works great, today, and nothing needs to be done here.

So far, there are no problems.

• Now, say the user creates an atom in this higher layer.
• Then the user "deletes" this atom. Should it be hidden, or actually deleted?
• Only the atomspace knows for sure, and it makes a decision to either hide or delete.
• Rocks should do exactly the same thing: hide, if the atomspace hides, or delete, if the atomspace deletes. But how can it do this?
• The two-stage pre- and post- callback hooks gives rocks a chance to think about things.

Right now, the code is written so everything works; there aren't any bugs (that I'm aware of) in any of this. The optimization is that maybe rocks can delete the atom, instead of marking it as hidden, if the atomspace deleted it.

I'm actually confused by the details; a unit test needs to be written to explore this more carefully, to see what actually happens in this mixed-frame deletion process. Something that demonstrates a situation where we can go "ah hah! See! The atom is marked hidden everywhere and since its hidden everywhere, we can actually nuke it forever." Maybe there's even a bug in here, somewhere, I dunno. That would be a good place to start.

[linas]

Part of the problem here is that so much time elapses, that I kind of forget what the issue is, and I think you do too, and so we kind of reset onto some wrong track and focus on the wrong thing. Having a unit test that actually shows what the actual problem is would make it a lot easier to think about all of this clearly.

[sikefield3]

If the user is NOT using frames, then delete works fine, and nothing needs to be done.

We could just ask if frames are used - say here - and if not, it works as before without pre-remove and post-remove (I'm not sure. Maybe this is already happening).

If the user is using frames, then there is the following scenario:

* User creates atom in the base space

* User pushes a frame (All atoms in the base frame are visible, but now become "read-only")

* User wishes to delete some atom (that exists in the base space). Since the base space is read-only, it cannot be actually deleted. Instead, it is marked "hidden". To the user, it looks as if it's deleted; its not, its still there in the base, its just marked invisible in the higher layer.  This works great, today, and nothing needs to be done here.

Maybe, one could check this in the same place or somewhere else.

So far, there are no problems.

* Now, say the user creates an atom in this higher layer.

* Then the user "deletes" this atom. Should it be hidden, or actually deleted?

* Only the atomspace knows for sure, and it makes a decision to either hide or delete.

* Rocks should do exactly the same thing: hide, if the atomspace hides, or delete, if the atomspace deletes. But how can it do this?

* The two-stage pre- and post- callback hooks gives rocks a chance to think about things.

That's what I wanted to do, see my earlier comment. At the moment, I can't see that I am missing something apart from the things mentioned above ...

[sikefield3]

Part of the problem here is that so much time elapses, that I kind of forget what the issue is, and I think you do too, and so we kind of reset onto some wrong track and focus on the wrong thing. Having a unit test that actually shows what the actual problem is would make it a lot easier to think about all of this clearly.

I agree. I thinks it's for the best to create a unit test before continuing working on bug itself.

[linas]

I think it's for the best to create a unit test before continuing working on bug itself.

Yes, please. This is just subtle enough that, unless I think hard about it for a good long while, I forget what the issue is, which means that finding the right fix becomes hard.