wukong-m2m / wukong-darjeeling

Darjeeling for WuKong
Other
20 stars 17 forks source link

We should not use database cache for discovery #51

Closed senatorjo closed 10 years ago

senatorjo commented 10 years ago

If we change location of sensor nodes and rediscover again, the same result will be shown. According to terminal dump, it seems database cache is used: [wkpfcomm] getting all nodes from cache

senatorjo commented 10 years ago

resolved by commit 0b5c5807cc98a39d0950f093c75922f0642f9ab1 in develop branch

senatorjo commented 10 years ago

Just found the fix will cause other problems, i.e. second time discovery will return all nodes will 0 wuclasses and 0 wuobjects. Need to dive more into the code....

senatorjo commented 10 years ago

Revered back to discovery through cache in commit 5192e44fdc433929ddf47d94d950ac84098345c8 .

The problem is basically because the database will assign its own ids to different nodes. Thus, during a second discovery, new node ids will be generated for the new discovery result, however, database content is not updated... Thus, master cannot find corresponding node in database and thought the nodes are not responding.

Eg. in the beginning, 5 nodes in database id 1,2,3,4,5. When we discover again, the same nodes will be given new ids, master will try to search in database 6,7,8,9,10 and found nothing. Thus, the result is incorrect.

senatorjo commented 10 years ago

This would be a big change in the database. Since I am not familiar with the database, especially the initialization and updating part, I am suggesting Penn to do it. Otherwise, I can do it in release 0.3 and remove the database.

wycc commented 10 years ago

No, we should not remove the database since I don't want the discovery is done in every reconfiguration. The cache should be used as much as possible.

Instead, we should update the cache whenever the device is changed. In this example, when we call Set location in the Node Editor, the cache should be updated as well. I believe that this is just a simple function call in the callback function. The same thing for include/exclude. In addition, we need to add a button in the node editor to invalidate the cache.

For now, the cache is invalidate when we restart the master. This is incorrect as well. I will open separate issue for those problems.

2013/11/5 senatorjo notifications@github.com

This would be a big change in the database. Since I am not familiar with the database, especially the initialization and updating part, I am suggesting Penn to do it. Otherwise, I can do it in release 0.3 and remove the database.

— Reply to this email directly or view it on GitHubhttps://github.com/wukong-m2m/wukong-darjeeling/issues/51#issuecomment-27724366 .

YC

Phone: 886-2-23516989 Mobile: 0919816155

wycc commented 10 years ago

Issue #96,#97,#98,#99 are added for the cache-related issues.

2013/11/5 Yu-Chung Wang wycc@homescenario.com

No, we should not remove the database since I don't want the discovery is done in every reconfiguration. The cache should be used as much as possible.

Instead, we should update the cache whenever the device is changed. In this example, when we call Set location in the Node Editor, the cache should be updated as well. I believe that this is just a simple function call in the callback function. The same thing for include/exclude. In addition, we need to add a button in the node editor to invalidate the cache.

For now, the cache is invalidate when we restart the master. This is incorrect as well. I will open separate issue for those problems.

2013/11/5 senatorjo notifications@github.com

This would be a big change in the database. Since I am not familiar with the database, especially the initialization and updating part, I am suggesting Penn to do it. Otherwise, I can do it in release 0.3 and remove the database.

— Reply to this email directly or view it on GitHubhttps://github.com/wukong-m2m/wukong-darjeeling/issues/51#issuecomment-27724366 .

YC

Phone: 886-2-23516989 Mobile: 0919816155

YC

Phone: 886-2-23516989 Mobile: 0919816155

senatorjo commented 10 years ago

@wycc It seems the reason we want to keep a database is just to use the cache, so maybe we still don't need the database. About in memory cache, we are already having this nodeinfo variable saving informations about everything. This is static for now because it's getting information from the cache of the database. So now we have three things, database => in memory cache of database => in memory nodeinfo. What I am talking about is instead of saving everything into the current static database and keep a in-memory copy of the database. we keep this in-memory nodeinfo data updated and maybe keep part of the original database data structure as well to ensure minimum changes. If we do it this way, we are not using the database at all. We could use database for collection of datas, but not for recording sensor status which requires a lot of update in some cases and may not require much consistency from master's perspective if we do some sort of lazy discovery.(like rediscover only part of the network only when master finds that part wrong).

The discovery is actually using in memory cache of database. I don't really understand if the database is updated or how the database is updated and that's the reason why I am more wiling to remove database and refractor this whole part of code than change the database. We can also save this in memory data in a xml so next time a previous discover result could be recovered(similar to simulation mode).That's why I said I can do this in r0.3, but for r0.2, I don't think I would have enough time to do either way given all the other jobs:)

wycc commented 10 years ago

No matter what we choice, we still need solve issue 96-99. Otherwise, whenever we add/delete/modify devices, we need to restart the master. I don't really care if the database is there or not. However, we need to make the flow reasonable and easy to understand. In my point of view, I only have the following requirement.

(1) The discovery is executed as little as possible. We may not need it at all. (2) When we add/delete/modify devices, the node info need to be updated immediately. (3) When we restart the master, we should use the in disk discovery result.

This is not a 0.3 feature. I don't want to ask others to restart the master whenever they add a device. This does not make sense.

2013/11/5 senatorjo notifications@github.com

@wycc https://github.com/wycc It seems the reason we want to keep a database is just to use the cache, so maybe we still don't need the database. About in memory cache, we are already having this nodeinfo variable saving informations about everything. This is static for now because it's getting information from the cache of the database. So now we have three things, database => in memory cache of database => in memory nodeinfo. What I am talking about is instead of saving everything into the current static database and keep a in-memory copy of the database. we keep this in-memory nodeinfo data updated and maybe keep part of the original database data structure as well to ensure minimum changes. If we do it this way, we are not using the database at all. We could use database for collection of datas, but not for recording sensor status which requires a lot of update in some cases and may not require much consistency from master's perspectiv e if we do some sort of lazy discovery.(like rediscover only part of the network only when master finds that part wrong).

The discovery is actually using in memory cache of database. I don't really understand if the database is updated or how the database is updated and that's the reason why I am more wiling to remove database and refractor this whole part of code than change the database. We can also save this in memory data in a xml so next time a previous discover result could be recovered(similar to simulation mode).That's why I said I can do this in r0.3, but for r0.2, I don't think I would have enough time to do either way given all the other jobs:)

— Reply to this email directly or view it on GitHubhttps://github.com/wukong-m2m/wukong-darjeeling/issues/51#issuecomment-27747715 .

senatorjo commented 10 years ago

Got you:)

senatorjo commented 10 years ago

In database, we should find if the discovered node ids are already there, if it is, replace it; if it it not, add a new entry for the node.

pencilcheck commented 10 years ago

I think maybe you can try to remove all entries in the database before every full discovery when user click on the discovery button? Otherwise use the existing discovery result from the database whenever possible.

So the only change would be to invalidate cache and reset database just before every full discovery, then this issue should be resolved. Sen can you try to do that? python sqlite should have method calls to clear up the database.