osgi / bugzilla-archive

Archive of OSGi Alliance Specification Bugzilla bugs. The Specification Bugzilla system was decommissioned with the move to GitHub. The issues in this repository are imported from the Specification Bugzilla system for archival purposes.
0 stars 1 forks source link

Indeterminate operations sequence when atomic events are handled #1594

Closed bjhargrave closed 13 years ago

bjhargrave commented 14 years ago

Original bug ID: BZ#1726 From: Evgeni Grigorov <e.grigorov@prosyst.com> Reported version: R4 V4.3

bjhargrave commented 14 years ago

Comment author: Evgeni Grigorov <e.grigorov@prosyst.com>

Currently, by specification: " Each event must carry the information of all nodes that underwent the related operation. • info/dmtree/DmtEvent/ADDED – New nodes were added. • info/dmtree/DmtEvent/DELETED – Existing nodes were removed. • info/dmtree/DmtEvent/REPLACED – Existing node values or other properties were changed. • info/dmtree/DmtEvent/RENAMED – Existing nodes were renamed. • info/dmtree/DmtEvent/COPIED – Existing nodes were copied. A copy operation does not trigger an ADDED event (in addition to the COPIED event), even though new node(s) are created. • info/dmtree/DmtEvent/SESSION_OPENED – A new sessions was opened. • info/dmtree/DmtEvent/SESSION_CLOSED – A session was closed (by means of the close operation or an error). For an atomic session, a maximum of five events can be sent: one for each operation type. In this case, the ordering for the events must follow the order of the previous list. " That specification section breaks the following use case:

  1. open atomic session on the node ./A
  2. delete node ./A/B
  3. add node ./A/B
  4. commit and close the session

The DMT events, which will be posted are: info/dmtree/DmtEvent/SESSION_OPENED { session.id=13 } info/dmtree/DmtEvent/ADDED { session.id=13 nodes=[./A/B] } info/dmtree/DmtEvent/DELETED { session.id=13 nodes=[./A/B] } info/dmtree/DmtEvent/SESSION_CLOSED { session.id=13 }

Is ./A/B node available or not?

According to the events, the node is deleted and should be missing. According to the operations sequence, the node is added and must be available.

The problem is in the specified event sequence and the nodes grouping. One possible solution, the event sequence must be according to the session operations i.e. if the operations are delete and add, then the events will be DELETED and ADD. The event nodes can be grouped only for the same consecutive operations i.e. if the operation are delete ./a/b, delete ./a/c and add ./a/d, then the events will be info/dmtree/DmtEvent/DELETED { session.id=13 nodes=[./a/b, ./a/c] } info/dmtree/DmtEvent/ADDED { session.id=13 nodes=[./a/d] } Another example: The operations are: delete ./a/b, add ./a/c, delete ./a/d The events will be: info/dmtree/DmtEvent/DELETED { session.id=13 nodes=[./a/b] } info/dmtree/DmtEvent/ADDED { session.id=13 nodes=[./a/c] } info/dmtree/DmtEvent/DELETED { session.id=13 nodes=[./a/d] } In this case, we cannot group the DELETED event nodes.

bjhargrave commented 14 years ago

Comment author: Andreas Kraft <a.kraft@telekom.de>

The DMT events, which will be posted are: ... According to the events, the node is deleted and should be missing. According to the operations sequence, the node is added and must be available.

Why is the order of the events the other way around? They should be -DELETED -ADDED because this is the order of the operation.

Just one question to clarify "atomic": With "atomic operation" you mean transactions, right?

With transaction we might have to clarify that events are not reliable to actually track the changes in the DMT, because a transaction might fail in the middle of a commit (I assume that in a transaction events are only raised in the commit). Events that have been raised before the fail cannot be taken back.

bjhargrave commented 14 years ago

Comment author: Evgeni Grigorov <e.grigorov@prosyst.com>

The DMT events, which will be posted are: ... According to the events, the node is deleted and should be missing. According to the operations sequence, the node is added and must be available.

Why is the order of the events the other way around?They should be This is specified by the Dmt Admin specification, see the paragraph above. -DELETED -ADDED because this is the order of the operation. Just one question to clarify "atomic": With "atomic operation" you mean transactions, right? Right, "atomic operation" - operation, which is executed in DmtSession of type LOCK_TYPE_ATOMIC i.e. in transaction

With transaction we might have to clarify that events are not reliable to actually track the changes in the DMT, because a transaction might fail in the middle of a commit (I assume that in a transaction events are only raised in the commit). Events that have been raised before the fail cannot be taken back. It will be useful for the event listeners like TR-069 Protocol Adapter.

bjhargrave commented 14 years ago

Comment author: Andreas Kraft <a.kraft@telekom.de>

This is specified by the Dmt Admin specification, see the paragraph above.

Ah, you mean:

"For an atomic session, a maximum of five events can be sent: one for each operation type. In this case, the ordering for the events must follow the order of the previous list."

hm, I don't understand this. This seems wrong to me. This even states that "OPEN" comes after e.g. ADDED. Wrong.

Right, "atomic operation" - operation, which is executed in DmtSession of type LOCK_TYPE_ATOMIC i.e. in transaction

Thanks, I just wanted to clarify. It seems to me that there is something missing in the original spec of transactions so far.

With transaction we might have to clarify that events are not reliable to actually track the changes in the DMT, because a transaction might fail in the middle of a commit (I assume that in a transaction events are only raised in the commit). Events that have been raised before the fail cannot be taken back. It will be useful for the event listeners like TR-069 Protocol Adapter.

Yes, informative, but not useful, because the PA would not receive all the changes and even wrongly, because the transaction, in case of failure, would rollback. But the events have already been raised. Would it help to specify that events need to be delayed until the end of a commit?

bjhargrave commented 14 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

This is specified by the Dmt Admin specification, see the paragraph above.

Ah, you mean:

"For an atomic session, a maximum of five events can be sent: one for each operation type. In this case, the ordering for the events must follow the order of the previous list."

hm, I don't understand this. This seems wrong to me. This even states that "OPEN" comes after e.g. ADDED. Wrong.

Andreas, you seems to misunderstand it. The paragraph says not "maximum of 7 events" but "maximum of 5 events": which implicitly means between SESSION_OPENED and SESSION_CLOSED, 5 events (ADDED/DELETED/REPLACED/RENAMED/COPIED) might be fired in the order of the list.

Right, "atomic operation" - operation, which is executed in DmtSession of type LOCK_TYPE_ATOMIC i.e. in transaction

Thanks, I just wanted to clarify. It seems to me that there is something missing in the original spec of transactions so far.

MEG might have some thought about it when they designed the spec. (I don't know it).

The problem is changing the spec will break the backward compatibility. Who can judge this chance is acceptable or not ?

With transaction we might have to clarify that events are not reliable to actually track the changes in the DMT, because a transaction might fail in the middle of a commit (I assume that in a transaction events are only raised in the commit). Events that have been raised before the fail cannot be taken back. It will be useful for the event listeners like TR-069 Protocol Adapter.

Yes, informative, but not useful, because the PA would not receive all the changes and even wrongly, because the transaction, in case of failure, would rollback. But the events have already been raised.

Would it help to specify that events need to be delayed until the end of a commit?

That is already written in the spec.


117.10.1 Event Admin based Events The Dmt Admin service uses the Event Admin service for event delivery. For atomic sessions, events are only sent at the time the session is committed (which can happen multiple times during a session). Otherwise they are sent immediately.

bjhargrave commented 14 years ago

Comment author: Andreas Kraft <a.kraft@telekom.de>

Ikuo, thanks for clarifying.

Andreas, you seems to misunderstand it. The paragraph says not "maximum of 7 events" but "maximum of 5 events": which implicitly means between SESSION_OPENED and SESSION_CLOSED, 5 events (ADDED/DELETED/REPLACED/RENAMED/COPIED) might be fired in the order of the list.

OK, the OPEN and CLOSED are left out. But does this really mean, that only 5 events are fired, e.g. "ADD, ADD, DELETE, ADD, ADD", even when there 25 operations and the number of events actually in a non-atomic session would result in 25 separate events? Or does this mean that only these 5 events must be fired, but the number of events fired could be greater than 5?

Sorry, but to limit the number of events fired in a atomic session is silly. And plain wrong. It would be better to not fire any events at all instead of raising the wrong number. And which 5 events should be fired? The first 5, the last 5, 5 in the middle, 5 the DMT Admin likes most?

MEG might have some thought about it when they designed the spec. (I don't know it).

The problem is changing the spec will break the backward compatibility. Who can judge this chance is acceptable or not ?

Can CPEG decide? I think that with the increased version number we can actually change this behaviour. It doesn't break an API, does it?

Would it help to specify that events need to be delayed until the end of a commit?

That is already written in the spec.


117.10.1 Event Admin based Events The Dmt Admin service uses the Event Admin service for event delivery. For atomic sessions, events are only sent at the time the session is committed (which can happen multiple times during a session). Otherwise they are sent immediately.

Ah, thanks. I haven't found this paragraph.

bjhargrave commented 14 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

Ikuo, thanks for clarifying.

Andreas, you seems to misunderstand it. The paragraph says not "maximum of 7 events" but "maximum of 5 events": which implicitly means between SESSION_OPENED and SESSION_CLOSED, 5 events (ADDED/DELETED/REPLACED/RENAMED/COPIED) might be fired in the order of the list.

OK, the OPEN and CLOSED are left out. But does this really mean, that only 5 events are fired, e.g. "ADD, ADD, DELETE, ADD, ADD", even when there 25 operations and the number of events actually in a non-atomic session would result in 25 separate events? Or does this mean that only these 5 events must be fired, but the number of events fired could be greater than 5?

The first one. The example in "117.10.1 Event Admin based Events" tells it. There are two addNode operations but the only one ADDED event with nodes =[./A/B/C, ./A/B/C/D]

I think it is intentionally designed in Dmt Admin Service Version 1.0 because of this example existence.

Sorry, but to limit the number of events fired in a atomic session is silly. And plain wrong. It would be better to not fire any events at all instead of raising the wrong number. And which 5 events should be fired? The first 5, the last 5, 5 in the middle, 5 the DMT Admin likes most?

MEG might have some thought about it when they designed the spec. (I don't know it).

The problem is changing the spec will break the backward compatibility. Who can judge this chance is acceptable or not ?

Can CPEG decide? I think that with the increased version number we can actually change this behaviour. It doesn't break an API, does it?

I had added meg-inbox@ www.osgi.org. Could you please tell your opinion, MEG guys ?

If there exists anyone who had implemented a Protocol Adapter or Local Manager, which received those DmtEvent and deal with them according to the description, it would break. However, I doubt there exists anyone.

Would it help to specify that events need to be delayed until the end of a commit?

That is already written in the spec.


117.10.1 Event Admin based Events The Dmt Admin service uses the Event Admin service for event delivery. For atomic sessions, events are only sent at the time the session is committed (which can happen multiple times during a session). Otherwise they are sent immediately.

Ah, thanks. I haven't found this paragraph.

bjhargrave commented 14 years ago

Comment author: Evgeni Grigorov <e.grigorov@prosyst.com>

The first one. The example in "117.10.1 Event Admin based Events" tells it. There are two addNode operations but the only one ADDED event with nodes =[./A/B/C, ./A/B/C/D]

I think it is intentionally designed in Dmt Admin Service Version 1.0 because of this example existence. In atomic session the operations are delayed to the commit. In this case, the events are buffered to the commit too. That buffer is a good reason to reduce the number of raised events. One possible solution is available in the current Dmt Admin specification. Apparently, it is not perfect and there is a bug.

Sorry, but to limit the number of events fired in a atomic session is silly. And plain wrong. It would be better to not fire any events at all instead of raising the wrong number. And which 5 events should be fired? The first 5, the last 5, 5 in the middle, 5 the DMT Admin likes most?

MEG might have some thought about it when they designed the spec. (I don't know it).

The problem is changing the spec will break the backward compatibility. Who can judge this chance is acceptable or not ?

Can CPEG decide? I think that with the increased version number we can actually change this behaviour. It doesn't break an API, does it?

I had added meg-inbox@ www.osgi.org. Could you please tell your opinion, MEG guys ?

If there exists anyone who had implemented a Protocol Adapter or Local Manager, which received those DmtEvent and deal with them according to the description, it would break. However, I doubt there exists anyone. Actually, the bug report was triggered by a implementation problem.

bjhargrave commented 14 years ago

Comment author: Gunnar Ekolin <ekolin@makewave.com>

Going back to the first example for an atomic session I think that the original MEG-spec would expect the following sequence of events:

The DMT events, which will be posted are: info/dmtree/DmtEvent/SESSION_OPENED { session.id=13 } info/dmtree/DmtEvent/REPLACED { session.id=13 nodes=[./A/B] } info/dmtree/DmtEvent/SESSION_CLOSED { session.id=13 }

Since the session is atomic the events should show how the tree is changed at the commit (no visible changes are allowed to take place before), i.e., the sum of changes and not each individual (potentially ovelapping) operation. Thus it should be enough to have exactly one event of each type posted for an atomic session.

bjhargrave commented 14 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

Going back to the first example for an atomic session I think that the original MEG-spec would expect the following sequence of events:

The DMT events, which will be posted are: info/dmtree/DmtEvent/SESSION_OPENED { session.id=13 } info/dmtree/DmtEvent/REPLACED { session.id=13 nodes=[./A/B] } info/dmtree/DmtEvent/SESSION_CLOSED { session.id=13 }

Since the session is atomic the events should show how the tree is changed at the commit (no visible changes are allowed to take place before), i.e., the sum of changes and not each individual (potentially ovelapping) operation. Thus it should be enough to have exactly one event of each type posted for an atomic session.

Do you mean the DmtAdmin compares the node structure at the beginning of the session and the node structure after the commit and judge which events should be sent (it does not the matter of the operation sequence). Regarding ADDED, DELETED, and REPLACED, it sounds reasonable. However, how about COPIED ? I don't think the DmtAdmin will do such an intelligent behavior.

bjhargrave commented 13 years ago

Comment author: Evgeni Grigorov <e.grigorov@prosyst.com>

As whole, atomic events are not correct. The general solution will be provided by Bug#1794.