Re: Review Request 37929: Changed provisioner Store API and implementation so it works as a read-through cache.

[Jiang Yan Xu]

> On Sept. 2, 2015, 5:53 p.m., Timothy Chen wrote:
> > src/slave/containerizer/provisioners/appc/store.cpp, line 235
> > 
> >
> > The wording sounds wierd, how about:
> > 
> > "Cannot match any image in store for '" + image.name()

Changed to:

```
return Failure("No image named '" + image.name() +
   "' can match the requirements");
```

Note that it's not about local availability anymore since the store is also 
responsible for retrieving it.


- Jiang Yan


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37929/#review97585
---


On Sept. 2, 2015, 5:07 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37929/
> ---
> 
> (Updated Sept. 2, 2015, 5:07 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Jie Yu, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - i.e., It fetches images transparently when it's not in the local cache.
> - This way, the store doesn't have the sense of "localness" anymore but 
> rather it's an abstraction that provides access to all discoverable images, 
> no matter where they can be found.
> 
> Some context for motivation of this change can be found at: 
> https://reviews.apache.org/r/37881/
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioners/appc/store.hpp 
> e48d91be06410bfc028a7b2ed88218e13adbffee 
>   src/slave/containerizer/provisioners/appc/store.cpp 
> fbd1c535d398a4d37c30ba23f5408095c7d35b65 
>   src/tests/containerizer/appc_provisioner_tests.cpp 
> 47b66b9c30cefe8f9a8e2c1c1341776c2d235020 
> 
> Diff: https://reviews.apache.org/r/37929/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 37929: Changed provisioner Store API and implementation so it works as a read-through cache.

[Jiang Yan Xu]

> On Aug. 31, 2015, 2:42 p.m., Chi Zhang wrote:
> > src/slave/containerizer/provisioners/appc/store.hpp, lines 76-81
> > 
> >
> > Maybe not yet the guarantee on ordering since we don't support 
> > dependency in images.
> > 
> > Mainly I think if we don't have to worry about ordering, it is simpler.
> > 
> > If backends can't get away from that, we should then be more explicit 
> > on what kind of ordering we provie. BFS, first order traversal or sth else?
> 
> Jiang Yan Xu wrote:
> This comment is about what the method SHOULD do. And the TODO below says: 
> OK we are actually not doing it just yet. :)
> 
> It's important to be clear about what we WILL do in this case because 
> this is the contract between the provisioner and the store. We need to 
> determine right now where the logic should go in order to make sure the 
> interface is future proof.
> 
> The backend is told about the ordering because it itself doesn't have 
> enough information to otherwise detect it.
> 
> The ordering is basically "what overwites what in the fact of conflict" 
> so it's not BFS or how we RESOLVE the dependencies.
> 
> Chi Zhang wrote:
> I guess I wasn't clear if C, B, D, A and C, D, B, A are both acceptable 
> in your example, afte reading the comment?
> 
> Jiang Yan Xu wrote:
> Thanks. You are right I could have been more clear about this. In fact B 
> needs to go before D so I added the following comment: 
> (B before D in this example is decided by the order in which A specifies 
> its dependencies)
> 
> Chi Zhang wrote:
> is it true that the order at which A speicifies dependecies matters?

If image A depends on [B, D], it may intentionally want some files in D to 
overwrite the ones in B, right?
I am not saying it's a good practice but it's the intention of the spec to 
allow it.


- Jiang Yan


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37929/#review97157
---


On Sept. 2, 2015, 5:07 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37929/
> ---
> 
> (Updated Sept. 2, 2015, 5:07 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Jie Yu, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - i.e., It fetches images transparently when it's not in the local cache.
> - This way, the store doesn't have the sense of "localness" anymore but 
> rather it's an abstraction that provides access to all discoverable images, 
> no matter where they can be found.
> 
> Some context for motivation of this change can be found at: 
> https://reviews.apache.org/r/37881/
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioners/appc/store.hpp 
> e48d91be06410bfc028a7b2ed88218e13adbffee 
>   src/slave/containerizer/provisioners/appc/store.cpp 
> fbd1c535d398a4d37c30ba23f5408095c7d35b65 
>   src/tests/containerizer/appc_provisioner_tests.cpp 
> 47b66b9c30cefe8f9a8e2c1c1341776c2d235020 
> 
> Diff: https://reviews.apache.org/r/37929/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 37929: Changed provisioner Store API and implementation so it works as a read-through cache.

[Chi Zhang]

> On Aug. 31, 2015, 9:42 p.m., Chi Zhang wrote:
> > src/slave/containerizer/provisioners/appc/store.hpp, lines 76-81
> > 
> >
> > Maybe not yet the guarantee on ordering since we don't support 
> > dependency in images.
> > 
> > Mainly I think if we don't have to worry about ordering, it is simpler.
> > 
> > If backends can't get away from that, we should then be more explicit 
> > on what kind of ordering we provie. BFS, first order traversal or sth else?
> 
> Jiang Yan Xu wrote:
> This comment is about what the method SHOULD do. And the TODO below says: 
> OK we are actually not doing it just yet. :)
> 
> It's important to be clear about what we WILL do in this case because 
> this is the contract between the provisioner and the store. We need to 
> determine right now where the logic should go in order to make sure the 
> interface is future proof.
> 
> The backend is told about the ordering because it itself doesn't have 
> enough information to otherwise detect it.
> 
> The ordering is basically "what overwites what in the fact of conflict" 
> so it's not BFS or how we RESOLVE the dependencies.
> 
> Chi Zhang wrote:
> I guess I wasn't clear if C, B, D, A and C, D, B, A are both acceptable 
> in your example, afte reading the comment?
> 
> Jiang Yan Xu wrote:
> Thanks. You are right I could have been more clear about this. In fact B 
> needs to go before D so I added the following comment: 
> (B before D in this example is decided by the order in which A specifies 
> its dependencies)

is it true that the order at which A speicifies dependecies matters?


- Chi


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37929/#review97157
---


On Sept. 3, 2015, 12:07 a.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37929/
> ---
> 
> (Updated Sept. 3, 2015, 12:07 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Jie Yu, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - i.e., It fetches images transparently when it's not in the local cache.
> - This way, the store doesn't have the sense of "localness" anymore but 
> rather it's an abstraction that provides access to all discoverable images, 
> no matter where they can be found.
> 
> Some context for motivation of this change can be found at: 
> https://reviews.apache.org/r/37881/
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioners/appc/store.hpp 
> e48d91be06410bfc028a7b2ed88218e13adbffee 
>   src/slave/containerizer/provisioners/appc/store.cpp 
> fbd1c535d398a4d37c30ba23f5408095c7d35b65 
>   src/tests/containerizer/appc_provisioner_tests.cpp 
> 47b66b9c30cefe8f9a8e2c1c1341776c2d235020 
> 
> Diff: https://reviews.apache.org/r/37929/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 37929: Changed provisioner Store API and implementation so it works as a read-through cache.

[Timothy Chen]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37929/#review97585
---

Ship it!


Ship It!


src/slave/containerizer/provisioners/appc/store.cpp (line 235)


The wording sounds wierd, how about:

"Cannot match any image in store for '" + image.name()


- Timothy Chen


On Sept. 3, 2015, 12:07 a.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37929/
> ---
> 
> (Updated Sept. 3, 2015, 12:07 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Jie Yu, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - i.e., It fetches images transparently when it's not in the local cache.
> - This way, the store doesn't have the sense of "localness" anymore but 
> rather it's an abstraction that provides access to all discoverable images, 
> no matter where they can be found.
> 
> Some context for motivation of this change can be found at: 
> https://reviews.apache.org/r/37881/
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioners/appc/store.hpp 
> e48d91be06410bfc028a7b2ed88218e13adbffee 
>   src/slave/containerizer/provisioners/appc/store.cpp 
> fbd1c535d398a4d37c30ba23f5408095c7d35b65 
>   src/tests/containerizer/appc_provisioner_tests.cpp 
> 47b66b9c30cefe8f9a8e2c1c1341776c2d235020 
> 
> Diff: https://reviews.apache.org/r/37929/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 37929: Changed provisioner Store API and implementation so it works as a read-through cache.

[Jiang Yan Xu]

> On Aug. 31, 2015, 2:42 p.m., Chi Zhang wrote:
> > src/slave/containerizer/provisioners/appc/store.hpp, lines 76-81
> > 
> >
> > Maybe not yet the guarantee on ordering since we don't support 
> > dependency in images.
> > 
> > Mainly I think if we don't have to worry about ordering, it is simpler.
> > 
> > If backends can't get away from that, we should then be more explicit 
> > on what kind of ordering we provie. BFS, first order traversal or sth else?

This comment is about what the method SHOULD do. And the TODO below says: OK we 
are actually not doing it just yet. :)

It's important to be clear about what we WILL do in this case because this is 
the contract between the provisioner and the store. We need to determine right 
now where the logic should go in order to make sure the interface is future 
proof.

The backend is told about the ordering because it itself doesn't have enough 
information to otherwise detect it.

The ordering is basically "what overwites what in the fact of conflict" so it's 
not BFS or how we RESOLVE the dependencies.


- Jiang Yan


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37929/#review97157
---


On Aug. 31, 2015, 10:45 a.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37929/
> ---
> 
> (Updated Aug. 31, 2015, 10:45 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Jie Yu, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - i.e., It fetches images transparently when it's not in the local cache.
> - This way, the store doesn't have the sense of "localness" anymore but 
> rather it's an abstraction that provides access to all discoverable images, 
> no matter where they can be found.
> 
> Some context for motivation of this change can be found at: 
> https://reviews.apache.org/r/37881/
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioners/appc/store.hpp 
> e48d91be06410bfc028a7b2ed88218e13adbffee 
>   src/slave/containerizer/provisioners/appc/store.cpp 
> fbd1c535d398a4d37c30ba23f5408095c7d35b65 
>   src/tests/containerizer/appc_provisioner_tests.cpp 
> 47b66b9c30cefe8f9a8e2c1c1341776c2d235020 
> 
> Diff: https://reviews.apache.org/r/37929/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 37929: Changed provisioner Store API and implementation so it works as a read-through cache.

[Jiang Yan Xu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37929/
---

(Updated Aug. 29, 2015, 9:25 p.m.)


Review request for mesos, Jie Yu and Timothy Chen.


Repository: mesos


Description
---

- i.e., It fetches images transparently when it's not in the local cache.
- This way, the store doesn't have the sense of localness anymore but rather 
it's an abstraction that provides access to all discoverable images, no matter 
where they can be found.

Some context for motivation of this change can be found at: 
https://reviews.apache.org/r/37881/


Diffs
-

  src/slave/containerizer/provisioners/appc/store.hpp 
e48d91be06410bfc028a7b2ed88218e13adbffee 
  src/slave/containerizer/provisioners/appc/store.cpp 
fbd1c535d398a4d37c30ba23f5408095c7d35b65 
  src/tests/containerizer/appc_provisioner_tests.cpp 
47b66b9c30cefe8f9a8e2c1c1341776c2d235020 

Diff: https://reviews.apache.org/r/37929/diff/


Testing
---

make check.


Thanks,

Jiang Yan Xu

Review Request 37929: Changed provisioner Store API and implementation so it works as a read-through cache.

[Jiang Yan Xu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37929/
---

Review request for mesos, Jie Yu and Timothy Chen.


Repository: mesos


Description
---

- i.e., It fetches images transparently when it's not in the local cache.
- This way, the store doesn't have the sense of localness anymore but rather 
it's an abstraction that provides access to all discoverable images, no matter 
where they can be found.

Some context for motivation of this change can be found at: 
https://reviews.apache.org/r/37881/


Diffs
-

  src/slave/containerizer/provisioners/appc/store.hpp 
e48d91be06410bfc028a7b2ed88218e13adbffee 
  src/slave/containerizer/provisioners/appc/store.cpp 
fbd1c535d398a4d37c30ba23f5408095c7d35b65 
  src/tests/containerizer/appc_provisioner_tests.cpp 
47b66b9c30cefe8f9a8e2c1c1341776c2d235020 

Diff: https://reviews.apache.org/r/37929/diff/


Testing
---

make check.


Thanks,

Jiang Yan Xu