Community
Participate
Working Groups
Build Identifier: Currently there is not possible to easy figure out the reason of existence for a profile timestamp. It would be helpful that the provisioning session that created the timestamp could be labeled/tagged as for example give it a name like "Install version 1.0" or "Upgrade to version 1.1". Reproducible: Always
Created attachment 178818 [details] Proposed patch The attached patch will make use on an profile property (defined in IProfile) to store the tag. Setting of profile property is one via a "-tag" option for director application. If tag is not set the property is removed from profile (timestamp). The paths is not complete as the tag may have to be set also from other profile changing apps like for example the UI (when installing/updating), from dropins (automatically set) or from installer app. Also it would be nice that the UI that shows history to also display the tag.
I have the two following commands on the director app: -tag -listTags and I have also modified the revert to also take a tag -revert One thing I wonder is whether the purge also needs to remove the tags or is this being dealt with by the implementation of the profile state storage. Which leads to a slightly related question, about what happens if ppl delete profiles from disk by hand.
I just took a look at the code to confirm behaviour. There is not profile state meta data clean up for free. So deleting a profile state does not delete it's associated meta data. Likewise, a user manually deleting profile states from the file system will not force a clean up of the profile state meta data. The net result that a call to IProfileRegistry.getProfileStateProperties(String id, long timestamp) for a timestamp that no longer exists but had meta data associated with it, will return meta data. Callers could conceivably run into problems as they probably don't have sufficient guard code to deal with meta data collections with "extra" elements. Although it would be arguably difficult for a caller to get a handle on a timestamp that no longer exists in the file system. The other problem, of course, is that the single profile meta data file would continue to grow. One possible solution would be to modify readStateProperties(String) and writeStateProperties(String, Properties) such that it will not read or write properties for timestamps that are not known by the IProfileRegistry. Currently the SimpleProfileRegistry implementation does not cache the timestamp list, so this change would mean that every read and write operation would do a directory listing and array sort on on the profile directory. Since a read is done on every get and set and a write is done on every set this may have unacceptable performance implications. Another option, would be do only do the timestamp check on write at longer intervals, once a week. This would fix the file bloat problem but callers to get would have to accept that they may receive meta data for timestamps that no longer exist.
> The net result that a call to IProfileRegistry.getProfileStateProperties(String > id, long timestamp) for a timestamp that no longer exists but had meta data > associated with it, will return meta data. Callers could conceivably run into > problems as they probably don't have sufficient guard code to deal with meta > data collections with "extra" elements. Although it would be arguably > difficult for a caller to get a handle on a timestamp that no longer exists in > the file system. > > The other problem, of course, is that the single profile meta data file would > continue to grow. > > One possible solution would be to modify readStateProperties(String) and > writeStateProperties(String, Properties) such that it will not read or write > properties for timestamps that are not known by the IProfileRegistry. > > Currently the SimpleProfileRegistry implementation does not cache the timestamp > list, so this change would mean that every read and write operation would do a > directory listing and array sort on on the profile directory. Since a read is > done on every get and set and a write is done on every set this may have > unacceptable performance implications. > > Another option, would be do only do the timestamp check on write at longer > intervals, once a week. This would fix the file bloat problem but callers to > get would have to accept that they may receive meta data for timestamps that no > longer exist. Given that we can't cover for the case where entries are deleted from disk, I think the first thing to do is to change the contract of the API to indicate that it is possible for the state properties API to return non existing entries. On the actual implementation side, I would go with a simple clearing of the old entries on write only. What do you think?
I just had an idea when reading your reply ... not sure why I didn't see this yesterday. If we do the check on the reads, then we don't actually have to redo the checks on the write. However, reads probably happen more often than writes and we would still have to change the contract since we still could not be 100% sure some file system deletions wouldn't sneak in the middle.... So I guess I agree with your proposal from the previous comment to do them on the writes ... unless you have strong feelings about this comment.
(In reply to comment #5) > I just had an idea when reading your reply ... not sure why I didn't see this > yesterday. > > If we do the check on the reads, then we don't actually have to redo the checks > on the write. However, reads probably happen more often than writes and we > would still have to change the contract since we still could not be 100% sure > some file system deletions wouldn't sneak in the middle.... > > So I guess I agree with your proposal from the previous comment to do them on > the writes ... unless you have strong feelings about this comment. For now, let's do it on writes.
Created attachment 189201 [details] Patch to prune profile state meta data for states that have been removed Released patch that prunes profile state meta data for states that have been removed (either via API or file system) each time the profile states are written. Note that the existing implementation of SimpleProfileRegistry.removeProfile(String profileID) takes care of the case where an entire profile with all its states are removed. Also update the javadoc for the API for the get methods to make it clear that properties for states that no longer physically exist may be returned. Since a user can delete profile states from the file system we have no good way to insure that only existing states are returned. But this new pruning code will prevent the state meta data files from filling with state data.
Patch released. Additionally I think we should make a call to #removeProfileStateProperties(id, timestamp, null) in the method #removeProfile(id, timestamp). Thoughts?
Seems reasonable to me
Created attachment 189226 [details] patch Patch which removes associated profile state properties when you remove a particular profile state.
Patch released to HEAD.
Thx for the patch. Closing.
