Skip to content

Commit

Permalink
doc: add TSC meeting notes 2015-06-10
Browse files Browse the repository at this point in the history
PR-URL: #1943
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
  • Loading branch information
piscisaureus committed Jun 17, 2015
1 parent 0abcf44 commit 30638b1
Showing 1 changed file with 183 additions and 0 deletions.
183 changes: 183 additions & 0 deletions doc/tc-meetings/2015-06-10.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
# Node Foundation TSC Meeting 2015-06-10

## Links

* **GitHub Issue**: https://github.com/nodejs/node/issues/53
* **Original Minutes Google Doc**: https://docs.google.com/document/d/1cn7SKaKYUMYLBiQhE6HAAknnwPabKsYjpOuyQkVikW8

## Agenda

Extracted from **tsc-agenda** labelled issues and pull requests prior to meeting.

### nodejs/io.js

* Add working group state per Project Lifecycle. #1880
* Chrome 43 released; time for V8 4.3! #1735
* TSC needs to elect a board representative. #1697

### joyent/node
* Nominating new collaborators to this repository #25481
* Node should not automatically change rlimits #8704
* Re-purpose previous joyent/node TSC meeting as LTS working group
* Releases for logjam - expect new openssl version soon (possibly should be discussed in LTS working group instead)

## Minutes

### Present

* Shigeki Ohtsu (TSC)
* Colin Ihrig (TSC)
* Alexis Campailla (TSC)
* Bert Belder (TSC)
* Ben Noordhuis (TSC)
* Julien Gilli (TSC)
* Mikeal Rogers
* Michael Dawson (TSC)
* Domenic Denicola
* Brian White (TSC)

This comment has been minimized.

Copy link
@Fishrock123

Fishrock123 Jun 17, 2015

Contributor

Brian and Shigeki also are not TSC yet, guess I'll have to make a PR..

* Bradley Meck
* Trevor Norris (TSC)

### Quick review of the previous meeting

* **Bert**: Discussed adding WG for project life cycle. Don’t remember what the idea was.
* **Mikeal**: Need to merge the associated PR.
* **Bert**: Discussed upgrading V8 to 4.3. Some contention, much in favor of doing it. Issue is V8 is changing API in 4.3 and 4.4 again.
* **Trevor**: Biggest issue is nan 2.0 hadn’t come out yet. There would be a gap in time when no native module would work.
* **Bert**: Resolution was that we would try to get nan to update asap. If 4.4 wasn’t done yet we would not upgrade to 4.3 yet.
* **Bert**: Mikeal had proposed to split meeting in two different meetings: one technical, one non technical.

### Standup

* **Shigeki**: preparing for the next OpenSSL update.
* **Michael**: working on triaging issues, made sure V8 beta/stable build properly on PPC. Internally setup some build to build io.js next on PPC, so that we can hook up build machines asap.
* **Colin**: mostly just reviewed PRs and triaged issues.
* **Brian**: mostly triaging issues, looked at some PRs.
* **Julien**: triaged issues, reviewed PRs. Started documenting Node.js’ release process and CI platform. Also started to document breaking changes between v0.12 and the converged repo between io.js and Node.js.
* **Alexis**: investigations on native addon WG.
* **Bert**: not a lot of technical work. Working on libuv changes to make the multi isolate/workers stuff work. PR #396 in libuv.
* **Ben**: reviewed PRs.
* **Domenic**: some reviews and tried to gear up for 3.0.
* **Trevor**: Lot of reviewing and landing of PRs, prototype for lower-level JS API. Worked on async listeners API.
* **Bradley**: here for rlimits discussion on the agenda.
* **Mikeal**: launch of the foundation, getting the PR ready and working on nodeconf that starts tomorrow.

### Today’s agenda

#### Add working group state per Project Lifecycle

* **Mikeal**: pretty much resolved, we just need to close the PR.

#### Chrome 43 released; time for V8 4.3

* **Domenic**: not sure if there’s much progress on that. Haven’t seen news from nan.
* **Bert**: remove tsc-agenda, someone should put back the tag when there’s news.

#### TSC needs to elect a board representative.

* **Mikeal**: thought that we had a resolution, and Bert and Rod would both be on the board for a short time.
* **Bert**: see if that can stay that way in the long term. Any objection?
* **Michael**: have we approached other people from the foundation to see if that’s possible?
* **Bert**: haven’t reached out yet. Who is organizing these meetings?
* **Mikeal**: Mike Dolan.
* **Bert**: Will send an email to see if that’s feasible.
* **Michael**: what about voting?
* **Mikeal**: we would need to put down just one person who has voting rights.
* **Michael**: having two present in meetings, only one who has the vote is fine by me.
* **Bert**: I don’t expect to be any kind of contention, trademark is probably the only one I can think of now.
* **Mikeal**: It seems that people don’t have a good idea of what will be decided during these meetings, so maybe we can just flip a coin for this short period of time, and then when we know more of what’s discussed in these meetings we can have a vote.
* **Michael**: maybe have some sort of alternance between two people in the beginning?
* **Bert**: I think it would still be good to have one person who will always be there.
* **Bert**: proposed resolution: let’s both (Rod and I) be there and then we’ll reevaluate later.

#### Nominating new collaborators to this repository

See https://github.com/joyent/node/issues/25481.

Main question was: should we add nominated collaborators who are not io.js collaborators too the “collaborators” team in the nodejs organization too?

Decision was that it might be confusing at first, so onboarding of these collaborators will be done first in joyent/node only, but they will conceptually be Node.js collaborators (as in collaborators in the nodejs GitHub organization). Shortly after they will be added as io.js collaborators by people familiar with the onboarding process at io.js.

#### Node should not automatically change rlimits

* **Bradley**: summary is in node v0.10. Node respected soft ulimit for fds. Soft limit is recommended limit, second one called hard limit which is an enforced limit. By default, OSX has a very low soft limit, and that’s the reason why this patch landed. However when running programs, they usually don’t change rlimits, and we started seeing failures after the patch landed. Argument for the side for keeping the patch, you don’t hear people complaining about being out of file descriptors.
* **Michael**: did patch change only for OSX?
* **Bradley**: did for all platforms. Hard limit becomes soft, soft limit are erased.
* **Michael**: fix means you don’t see errors?
* **Bradley**: yes, but it has undesirable side effects. Traditionally when you limit resources, you set soft limits to what you want your process to be able to get. Node started to ignore those, so we started seeing things like fds leaks not being caught, etc.
* **Ben**: why wouldn’t you set the hard limit to the soft limit?
* **Bradley**: This patch completely erases your soft limit, can’t recover it. No option to use soft limits at this point. Cluster would want very high limit, workers would want lower limit. Using soft limit is usually how people do that. You can use hard limits, but once you do that you’re talking about absolute enforcement.
* **Bert**: proposal is to revert the patch?
* **Bradley**: couple options. Ideal option to me is to revert the patch. I would expect UNIX programs to leave my settings alone. I would at least expect an option to recover the soft limits. Having some way to pass settings via command line maybe?
* **Bradley**: either command line flag or environment variable. Problem with command line anything that spawns with something like fork would ignore it.
* **Bert**: actually happy that the patch landed. Just works solution seems better, I’ve seen so many problems in production. Two concerns: agree this is a bit weird that node changes limits of child processes. Would not be opposed to use an environment variable.
* **Bradley**: argument will come from sysadmins, not people not familiar with EMFILE and rlimits. Usually you see the EMFILE error, you request to someone to bump this limit and the problem is solved.
* **Trevor**: what if we added process.resetRlimits and when that process spawns it resets rlimits.
* **Bradley**: need to pass that to child processes still.
* **Trevor**: if using cluster, then that seems easy.
* **Bert**: Don’t like having globals for that kind of stuff, related to multi isolates. Maybe add as an option to child_process.spawn?
* **Trevor**: how would that help regarding multi isolates environments?
* **Bert**: Yeah, not suggesting to be able to set different limits per isolate. Ben any opinion?
* **Ben**: Resetting rlimits when we spawn new process would be fine. Don’t care about environment variable that would make node not touch rlimits.
* **Domenic**: Does node change system wide rlimits?
* **Bradley**: that depends on how the node process lifecycle.
* **Domenic**: Is there any way to ignore rlimits while not changing system wide rlimits?
* **Bradley**: it’s a per process setting. Want a way to keep limits to get warnings about file descriptors, and a way to recover limits in child processes. I can write a patch for that.
* **Ben**: Keep in mind you’ll probably need to update libuv in some way. Forking takes places in libuv.
* **Bert**: summary: node will restore rlimits on spawn.Other thing would be command line switch or environment variable.
* **Ben**: How are you going to use that? Are you using sandbox?
* **Bradley**: Not using sandbox. When I see EMFILE, I can set a warning that notifies me when things go crazy.

##### Resolution

* The current (rlimit-changing) behavior will be maintained.
* Node/libuv should restore the ulimit for the child processes it spawns.
* Support for leaving the ulimit untouched by setting an an environment variable will be added.
* Bradley Meck is going to create patches to node and libuv for this.

#### New OpenSSL version about to be released, with fixed for logjam

* **Michael**: will need a v0.10 and v0.12 release when OpenSSL release comes out.

#### Repurpose previous node.js TSC meeting as LTS WG meeting

* **Julien**: timing might not work for anyone.
* **Bert**: Rod pinged a bunch of people, haven’t seen a lot of response. Wyatt Preul is involved, you will probably want to reach out to him, Same for ofrobots.
* **Michael**: want to make sure that everyone is ok with merging LTS meeting with joyent/node TSC meeting.
* **Julien**: discussion ongoing here: https://github.com/nodejs/LTS/issues/6#issuecomment-109352447.

### Discussion items not on the agenda

#### 3.0 upgrade documentation

* **Trevor**: haven’t seen any doc on how to upgrade to 3.0. Should be put notes about breaking changes there, or somewhere else?
* **Domenic**: hard part is C++ side changes. Everything else should be handled by semver-major/minor labels.

### Rebasing on top of the 'next' branch (not on the agenda)

* **Trevor**: rebase of next on master is going to be fairly broken. Is there somebody who usually takes those merges?
* **Ben**: Not sure I understood.
* **Trevor**: Lots of fixes that went into 2.x will not merge cleanly in next. Some of the fixes are minor. Fix in zlib that uses kMaxLength.
* **Ben**: what’s the question?
* **Trevor**: wondering who’s responsible for the merge? Maybe I can help with that?
* **Ben**: most of the time I do that. I usually create a PR and ask people to give a quick sign off on it.

##### Action items

* Trevor will document C++ side changes.

#### Workers (multi-isolate) PR

* **Bert**: decided to take a look at the multi isolates patch. I know that Ben tried to review it and went half-way through.
* **Trevor**: tried twice now and haven’t managed to go all the way through.
* **Bert**: proposed solution is to just make sure it doesn’t break anything major, put it behind a flag and review it in more details later.
* **Trevor**: a lot of stylistic issues, and haven’t got any response.
* **Ben**: petka mentioned he’s quite busy at the moment. Maybe someone could address these comments?
* **Bert**: ping petka, maybe we can land it in small increments. Do we want it to be perfect before it lands?
* **Ben**: not comfortable landing code that hasn’t been reviewed.
* **Trevor**: nice thing is that there’s a lot of tests. Didn’t see anything invasive that would make things difficult later. One change that I don’t understand 100%. I would depend on someone else to give the OK.
* **Bert**: interesting and valuable change, it needs work to get landed.

## Next meeting

Will be held on June 17th, 1pm PST.

0 comments on commit 30638b1

Please sign in to comment.