Skip to content

Included partial updates for long running jobs on the updated code.#2

Open
namrata-simha wants to merge 2 commits intomichaelsevilla:masterfrom
namrata-simha:master
Open

Included partial updates for long running jobs on the updated code.#2
namrata-simha wants to merge 2 commits intomichaelsevilla:masterfrom
namrata-simha:master

Conversation

@namrata-simha
Copy link

michaelsevilla and others added 2 commits April 20, 2017 21:54
@michaelsevilla
Copy link
Owner

Thanks! I'm going to start working through this.

Copy link
Owner

@michaelsevilla michaelsevilla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome. Thanks. The code looks good and you knew what you were doing. :)

Overall, I was a little confused about some of the lines that you added that didn't look related to your project. Did you cp files from the docker container to your Ceph branch? Then did you issue the pull request from there? Because all your code is based off my master branch even though that branch didn't have CudeleFS stuff.

If this was the case, we need to rebase your code off the "CudeleFS" branch. Let me know and we can start figuring this out. It may take a while but I really want this code. ;)

<< " --inode=<integer>\n"
<< " --type=<UPDATE|OPEN|SESSION...><\n"
<< " --frag=<ino>.<frag> [--dname=<dentry string>]\n"
<< " --alternate-pool=pool-name\n"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these your changes? They look unrelated to your project.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These aren't my changes.

rank_str = "0";
}

dout(10) << "JournalTool::main rank_str=" << rank_str << dendl;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again -- this looks unrelated. I'm thinking you copied code from the docker container and then issued the pull request. So its confusing as to what your code is versus mine and the Ceph guy's.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I don't know what this is either. If you want, I can mark the code that I've written with comments or such for you to know what I've written. My code is mostly localized to the area where I have commented from where I have set an update interval, as you have mentioned below:

start = std::clock();
//spawn child process for background update
pid_t pid = fork();
if(pid == 0){
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. This is a good way to do this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, as you said, all your changes also seem to have come into my commit because your Ceph branch didn't have any of the Cudele stuff. Please tell me if there is a better way to do this to fix this issue.

}

// write all events to disk (without a header)
events_b1.write_file(file.c_str());
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that the file is sitting on disk, how does it get merged into the metadata server?

This wasn't a requirement for the project -- as I can write a little script that does it. I was just wondering if you had implemented somewhere.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have done the merges to the metadata server in the child process, as in what comes after the previous code snippet.

@namrata-simha
Copy link
Author

Yes, before I committed, I had a little bit of that confusion, because I did cp files from the docker container to your Ceph branch and then issue a pull request, as you said, and the commit showed a lot more lines than I had originally written. I wasn't sure what was yours and what was new, so I didn't know what to remove and what not to. Should I have done it in another way?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants