storage: transactional, new storage with transactional capabilities #1006
Conversation
…WIP) Signed-off-by: Máximo Cuadros <mcuadros@gmail.com>
|
What would be the relation of this storage with our current |
| return err | ||
| } | ||
|
|
||
| return c.ConfigStorer.SetConfig(cfg) |
There was a problem hiding this comment.
This might not be transactional depending on the ConfigStorer implementation under the hood. Per example, using filesystem implementation SetConfig can fail on Write, causing a partial write.
There was a problem hiding this comment.
Indeed, this can be a problem. At least for our use case, since we don't do a commit per operation, but after a group of operations. That is, we want a Commit on the full storage, not just part of it.
| @@ -0,0 +1,63 @@ | |||
| package transactional | |||
There was a problem hiding this comment.
No Commit method here, when the objects from temporal are copied to the main storer?
| return obj, err | ||
| } | ||
|
|
||
| func (o *ObjectStorage) IterEncodedObjects(t plumbing.ObjectType) (storer.EncodedObjectIter, error) { |
There was a problem hiding this comment.
Mixed concepts here. If we are talking about transactional object storage, objects shouldn't be read until Commit is called.
Also, depending on the use case, maybe we want to do the opposite, a Rollback.
There was a problem hiding this comment.
It should be ok to read objects in a transaction. Just like in SQL. As long as read in a transaction sees writes in the same transaction, and does not read objects written in a different transaction.
|
None, most likely in v5, the old transaction should be deleted. |
Signed-off-by: Máximo Cuadros <mcuadros@gmail.com>
| // deleted, remaining references at this maps are going to be deleted when | ||
| // commit is requested, the entries are added when RemoveReference is called | ||
| // and deleted if SetReference is called. | ||
| deleted map[plumbing.ReferenceName]struct{} |
There was a problem hiding this comment.
what do you think about this @smola?
There was a problem hiding this comment.
Sounds good.
However, I'm still concerned that the whole approach is not really usable by borges. If that was the goal, it would actually need its own implementation of Commit...
There was a problem hiding this comment.
Yes, the implementation of Commit should be ad-hoc for each interface, but I don't think is going to be the current one.
| deleted map[plumbing.ReferenceName]struct{} | ||
| // packRefs if true PackRefs is going to be called in the based storer when | ||
| // commit is called. | ||
| packRefs bool |
…ndSetReference Signed-off-by: Máximo Cuadros <mcuadros@gmail.com>
Signed-off-by: Máximo Cuadros <mcuadros@gmail.com>
mcuadros
changed the title
|
@smola @ajnavarro PTAL |
|
I'm thinking about how this fits in borges. It looks like it doesn't, since it would produce just additional copies of everything in filesystem for no additional benefit. |
|
I don't understand several things:
|
It does not need to work with current implementation, but I thought this was meant to be used by borges future implementation. So I would expect the result to be equal or better in terms of performance, robustness, etc.
Well, I have looked better to it and it looks like additional copies would not be necessary when borges provides a Commit implementation. It currently looks like this:
This could be, in theory, optimized as follows:
With this new implementation, if we integrated it with borges, I see it being something like this:
Would it be like this? Looks like it could actually work well with the custom Commit for borges, not sure if I'm missing something. |
|
@mcuadros What about providing an interface too? |
Signed-off-by: Máximo Cuadros <mcuadros@gmail.com>
This PR introduces a new storage, called transactional allow to do Rollback any kind of operation over the storage, discarding Objects, Config, References, etc