Details
-
Improvement
-
Status: Accepted
-
Major
-
Resolution: Unresolved
-
None
-
None
-
None
Attachments
Activity
This JIRA refers to this TODO from benjaminhindman: https://github.com/apache/mesos/blob/0.22.1/src/linux/cgroups.cpp#L64
Furthermore, idownes suggests that "These three variations on mount information should be consolidated and moved to stout, along with mount and umount.": https://github.com/apache/mesos/blob/master/src/linux/fs.hpp#L40
Perhaps one of them would like to Accept this issue and volunteer as Shepherd?
It would also be great if we turned the pseudo-doxygen formatting into proper doxygen/javadoc style.
Can you please be more elaborate on this issue.?There is no file called linux/fs.h in mesos 0.26 version. Is the issue resolved?
xds2000 I think just move the code in ./src/linux/fs.hpp and ./src/linux/cpp to ./3rdparty/libprocess/3rdparty/stout/include/stout/fs.hpp. Note that we support both windows and posix in stout/fs.hpp. And all functions related to ./src/linux/fs.hpp are only available in Linux. Maybe add something like stout/linux/fs.hpp or add them to stout/posix/fs.hpp
After look other exists code under src/linux/, e.g. perf.hpp, ns.hpp I think remove mesos::internal:: should be enough.
xds2000 I add my basic review comments on it now. For further review, you could send email to dev mailing list and find a committer shepherd you in this ticket.
have no linux box to testing on it. please hold on on patched code update.
xds2000 Could you add some context and goal in the description section? The title also doesn't seem to make sense.
Beyond that, what are we trying to accomplish here? Does this ticket allow something that we couldn't do before? Are we trying to make these filesystem utilities public...? Based on adam-mesos said above, are we trying to consolidate something into stout?
I'd be up for shepherding this, but I don't understand what we're trying to do.
Michael Hi,
Thanks for your guide, i have update based on above comments recap. i have update the description. I think the changed is reason with consolidated with other file's convention.
haosdent mcypark could you please reive it again.https://reviews.apache.org/r/45500/
Thank you very much for your patch, it looks to me basically besides I am not sure whether keep it in current folder or move it to stout.
i think the file convention is follow the current folder. if move it to stout, also need consider what benefit on the action. Could you please list some concerns let me understand your think?
xds2000 I think the reason to keep it in current folder is those functions now only can be used in Linux, just like src/linux/ns.hpp. But if we want to support them in osx or windows in the future, I think move to stout would be better because it places the generic utilities.
sure. i think this is another purpose to handle not this issue's concerns. Maybe the Shepherd can guide it. let it go.
xds2000 It looks like this patch is simply moving everything in mesos::internal::fs to fs. That is, it doesn't look like we're consolidating multiple implementations. Again, what are we trying to accomplish here? Is there some place where we want access to these functions but for some reason, are not accessible or something?
Thanks mcypark, i guess the mesos::internal prefix namespace is not suitable for fs. this patch is based on above comments. the original TODO's information is very confuse on me.
ping adam-mesos idownes benjaminhindman could you please give some comments?
hi, anyone can guide the reason to moving the linux/fs.hpp? i actually don't know what reason, just fee this is newbie issue, and follow the comments to do it. but mcypark mentioned above, we need a reasonable demand to answer the quesiton first. if not, please remove the ACCEPT status thanks.
xds2000, sorry for the confusion! And thanks for your patch
I think that this issue still makes sense - mcypark, I don't think the intention was to consolidate implementations. Rather, as Deshi suggested, I think the TODO indicates that a namespace nested within mesos:: doesn't make sense for a header which provides generic Linux functionality that isn't Mesos-specific. Looking at the namespaces declared within src/linux/, it's not entirely consistent, but in most cases we do not use the mesos:: prefix.
Deshi, if you want to pick this back up, we could try to find a shepherd for the ticket and continue. If you're busy with other things and don't have time, would you mind discarding the review request? We can always reopen it in the future.
Can anyone advice me a little on fixing this issue...