Code Review 2
This round of code reviews will proceed much like the reviews for Project 1. For each review, the project developers will present a portion of their design in class and other students will comment on the design and offer suggestions for improvement. Each presenting team will also receive written code reviews from other students in the class. All teams will receive a written code review from me, and I will hold half-hour meetings with each team to go over my review.
Presenters
To start off each code review, the presenting team will spend 10-15 minutes to address the following issues:
- For each of the classes under discussion, describe the key idea(s) behind that class. In particular, describe the information and/or design decisions that are hidden within that class.
- If you considered alternate designs, say what the alternatives were and how you chose between them.
- Show the specific method interfaces for the class and discuss briefly in order to give the audience a feel for what are the pieces within the class.
- Walk through an example of usage, such as the life cycle of a client request.
- Talk briefly about what you found hard and easy in designing your system.
- What are the elements of your design that you are happiest about? What are its weaknesses? Ideally, you will already be aware of all of the issues discovered by reviewers.
You should prepare slides for your presentation. Remember the overall goal is abstraction: finding a simple way to think about something that is internally complicated. Try to find a simple way to explain your design, so that even people who have not read the code can easily get the basic idea; think about what are the most important elements of your design. Of course, if there are tricky elements, you will also need to mention those as well (don't pretend something is simple when it really isn't).
Reviewers
Each project will be reviewed by four students from other projects, according to the table below. Reviewers must read over the relevant code before the given class and prepare a written code review on GitHub. To do this, go to the project in GitHub, click on the "Pull requests" tab, click on the "Project2" pull request, and click on the "Files changed" tab. Click on a line on the right side to enter comments for that line, then select "Start a review". You can download the project code from GitHub if you'd like to use your favorite IDE to read through it (all of the projects will be readable to everyone in the class). You don't need to read the entire project (though you are welcome to if you wish); you only need to read the parts related to the area being reviewed.
Enter comments on GitHub, but do not submit your review yet. The comments will be saved and you will be able to see them, but no one else will see them until you submit the review. After we have discussed the relevant code in class, then you should submit your review. I suggest providing 10-20 comments, depending on the complexity of the code you are reviewing and the number of useful issues you can identify (but don't invent issues if you can't find 10 meaningful things to comment on).
As you did in the Project 1 reviews, look for red flags in the code you review, and also consider the following overall questions:
- Are classes and methods deep? Does each entity have a simple interface? Does it do one thing completely and well, or are its responsibilities muddy? Is there information leakage? Are the interfaces as general-purpose as possible?
- Is the code obvious (everything immediately makes sense, and your first guesses about behavior turn out to be correct)? When things aren't obvious, think about why that is the case, and how they could be made more obvious.
Review Presentations
Two of the reviewers for each project will present their code reviews in class, after the team has introduced its design. You will have 5 minutes for your presentation. This will not be enough time for you to discuss your review in detail, so pick out the most interesting and important issues. Use slides to structure your presentation and display examples. Start by saying what you liked best about the design, and what you think is the biggest opportunity for improvement. Then describe a few of the most important red flags that you found; be specific, and show examples. If you identified solutions to problems, pick the one that is most interesting and tell us about it.
Review Topics
Here are the topics we will be reviewing in this round of reviews. Each code review will focus on one of these topics.
Persistence
Review the mechanisms for persistence, including both server state and the log, and how these mechanisms are used by the Raft request handling logic. How does the log mechanism handle very large logs? Ideally there should be no operations that require time or memory space proportional to the length of the entire log. Are the persistence APIs general-purpose or Raft-specific?
Networking Revisions
Evaluate the RPC communication infrastructure and compare it with the structure from Project 1. In what ways is the new design better (or worse) than Project 1? Is the new design general-purpose? Is there a single mechanism that can support both client-server and server-server communication?
Reviewers: see the
changes
file for the developers' view of what changed; you may
find it useful to clone the repo and checkout the project1
and project2
branches for easier comparison.
Client Command Lifecycle
Review the lifecycle of a client command, including how it is read by the client and transmitted to the server, how it gets into the server's log, when and how it gets applied, and how responses get returned to the client and printed. Also, review how commands get applied on follower machines. You do not need to review the details of log replication. What happens if a server gains or loses leadership while it is in the middle of replicating a client command?
Raft State Machine Revisions
Evaluate the structure of the Raft state machine (not the application state machine) and compare it with the structure from Project 1. In what ways is the new design better (or worse) than Project 1? Did switching from a state-centric design to a message-centric design make things simpler and/or more obvious? Are there other issues with the Project 2 structure that you can identify?
Reviewers: see the
changes
file for the developers' view of what changed; you may
find it useful to clone the repo and checkout the project1
and project2
branches for easier comparison.
Schedule
Date | Topic | Presenters (Repo) | Review&Present | Review |
Wed. 3/1 | Persistence | Manya/Akram (raft9) | Shreya, Elliot | Luca, Jerry |
Persistence | Michelina/Anna (raft1) | Chloe, Kevin | Shashank, George | |
State Machine Revisions | Briana/Alex (raft7) | Will, Cooper | Kathy, Chendi | |
Fri. 3/3 | Client Command Lifecycle | Shashank/Elliot (raft5) | Kathy, Briana | Will, Akram |
Client Command Lifecycle | Kevin/Jerry (raft3) | Luca, Michelina | Chloe, Alex | |
Networking Revisions | George/Cooper (raft2) | Chendi, Anna | Shreya, Manya |