{"action":"created","comment":{"url":"https://api.github.com/repos/m3db/m3ninx/pulls/comments/184109376","pull_request_review_id":115237490,"id":184109376,"diff_hunk":"@@ -22,224 +22,271 @@ package mem\n \n import (\n \t\"errors\"\n-\t\"fmt\"\n \tre \"regexp\"\n \t\"sync\"\n \n \t\"github.com/m3db/m3ninx/doc\"\n \t\"github.com/m3db/m3ninx/index\"\n \tsgmt \"github.com/m3db/m3ninx/index/segment\"\n+\t\"github.com/m3db/m3ninx/index/segment/mem/idsgen\"\n \t\"github.com/m3db/m3ninx/index/util\"\n \t\"github.com/m3db/m3ninx/postings\"\n )\n \n var (\n-\terrSegmentSealed     = errors.New(\"segment has been sealed\")\n+\terrDuplicateID       = errors.New(\"a batch cannot contain duplicate IDs\")\n \terrUnknownPostingsID = errors.New(\"unknown postings ID specified\")\n )\n \n+// nolint: maligned\n type segment struct {\n-\topts      Options\n \toffset    int\n \tnewUUIDFn util.NewUUIDFn\n \n-\t// Internal state of the segment. The allowed transitions are:\n-\t//   - Open -> Sealed -> Closed\n-\t//   - Open -> Closed\n \tstate struct {\n \t\tsync.RWMutex\n-\n-\t\tsealed bool\n \t\tclosed bool\n \t}\n \n-\t// Mapping of postings list ID to document.\n+\t// Mapping of postings ID to document.\n \tdocs struct {\n \t\tsync.RWMutex\n-\n \t\tdata []doc.Document\n \t}\n \n-\t// Current writer and reader IDs. Writers increment the writer ID for each new\n-\t// document and only increment the reader ID after the document has been fully\n-\t// indexed by the segment. Readers do not need to acquire a lock since the writers\n-\t// only increment the readerID after they have finished inserting a new document.\n-\t// TODO(jeromefroe): Reevaluate using a big lock here which only permits a single\n-\t// writer at a time.\n-\tids struct {\n-\t\tsync.RWMutex\n+\t// Mapping of term to postings list.\n+\ttermsDict termsDictionary\n \n-\t\twriter, reader postings.AtomicID\n+\twriter struct {\n+\t\tsync.Mutex\n+\t\tidSet  *idsgen.Map\n+\t\tnextID postings.ID\n \t}\n-\n-\t// Mapping of field (name and value) to postings list.\n-\ttermsDict termsDictionary\n+\treaderID postings.AtomicID\n }\n \n // NewSegment returns a new in-memory mutable segment. It will start assigning\n-// postings IDs at offset+1.\n+// postings IDs at the provided offset.\n func NewSegment(offset postings.ID, opts Options) (sgmt.MutableSegment, error) {\n \ts := &segment{\n-\t\topts:      opts,\n-\t\toffset:    int(offset) + 1, // The first ID assigned by the segment is offset + 1.\n+\t\toffset:    int(offset),\n \t\tnewUUIDFn: opts.NewUUIDFn(),\n \t\ttermsDict: newTermsDict(opts),\n+\t\treaderID:  postings.NewAtomicID(offset),\n \t}\n \n \ts.docs.data = make([]doc.Document, opts.InitialCapacity())\n \n-\ts.ids.writer = postings.NewAtomicID(offset)\n-\ts.ids.reader = postings.NewAtomicID(offset)\n+\ts.writer.idSet = idsgen.New(256)\n+\ts.writer.nextID = offset\n \treturn s, nil\n }\n \n-func (s *segment) Insert(d doc.Document) error {\n+func (s *segment) Insert(d doc.Document) ([]byte, error) {\n \ts.state.RLock()\n \tdefer s.state.RUnlock()\n \tif s.state.closed {\n-\t\treturn sgmt.ErrClosed\n-\t}\n-\n-\tif s.state.sealed {\n-\t\treturn errSegmentSealed\n+\t\treturn nil, sgmt.ErrClosed\n \t}\n \n-\tdocID, err := d.Validate()\n-\tif err != nil {\n-\t\treturn err\n-\t}\n+\t{\n+\t\ts.writer.Lock()\n \n-\tif docID != nil {\n-\t\tok, err := s.termsDict.ContainsTerm(doc.IDReservedFieldName, docID)\n-\t\tif err != nil {\n-\t\t\treturn fmt.Errorf(\"could not lookup document ID '%s'\", docID)\n-\t\t}\n-\t\tif ok {\n-\t\t\t// Index already contains this document.\n-\t\t\treturn nil\n-\t\t}\n-\t} else {\n-\t\tuuid, err := s.newUUIDFn()\n-\t\tif err != nil {\n-\t\t\treturn err\n+\t\tb := index.NewBatch([]doc.Document{d})\n+\t\tif err := s.prepareDocsWithLocks(b); err != nil {\n+\t\t\treturn nil, err\n \t\t}\n-\t\td.Fields = append(d.Fields, doc.Field{\n-\t\t\tName:  doc.IDReservedFieldName,\n-\t\t\tValue: uuid,\n-\t\t})\n-\t}\n \n-\t// TODO: Consider supporting concurrent writes by relaxing the requirement that\n-\t// inserted documents are immediately searchable.\n-\ts.ids.Lock()\n+\t\t// Update the document in case we generated a UUID for it.\n+\t\td = b.Docs[0]\n \n-\tnewID := s.ids.writer.Inc()\n-\ts.insertDoc(newID, d)\n-\terr = s.insertTerms(newID, d)\n-\ts.ids.reader.Inc()\n+\t\ts.insertDocWithLocks(d)\n+\t\ts.readerID.Inc()\n \n-\ts.ids.Unlock()\n+\t\ts.writer.Unlock()\n+\t}\n \n-\treturn err\n+\treturn d.ID, nil\n }\n \n-func (s *segment) Reader() (index.Reader, error) {\n+func (s *segment) InsertBatch(b index.Batch) error {\n \ts.state.RLock()\n+\tdefer s.state.RUnlock()\n \tif s.state.closed {\n-\t\ts.state.RUnlock()\n-\t\treturn nil, sgmt.ErrClosed\n+\t\treturn sgmt.ErrClosed\n \t}\n \n-\tmaxID := s.ids.reader.Load()\n-\tr := newReader(s, maxID)\n-\treturn r, nil\n-}\n+\tvar err error\n+\t{\n+\t\ts.writer.Lock()\n \n-func (s *segment) Seal() error {\n-\ts.state.Lock()\n-\tif s.state.sealed {\n-\t\ts.state.Unlock()\n-\t\treturn errSegmentSealed\n+\t\terr = s.prepareDocsWithLocks(b)\n+\t\tif err != nil && !index.IsBatchPartialError(err) {","path":"index/segment/mem/segment.go","position":179,"original_position":179,"commit_id":"55c2e2c2db9e6bd51de4077a4ae9e745c79db1db","original_commit_id":"55c2e2c2db9e6bd51de4077a4ae9e745c79db1db","user":{"login":"prateek","id":199982,"avatar_url":"https://avatars3.githubusercontent.com/u/199982?v=4","gravatar_id":"","url":"https://api.github.com/users/prateek","html_url":"https://github.com/prateek","followers_url":"https://api.github.com/users/prateek/followers","following_url":"https://api.github.com/users/prateek/following{/other_user}","gists_url":"https://api.github.com/users/prateek/gists{/gist_id}","starred_url":"https://api.github.com/users/prateek/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/prateek/subscriptions","organizations_url":"https://api.github.com/users/prateek/orgs","repos_url":"https://api.github.com/users/prateek/repos","events_url":"https://api.github.com/users/prateek/events{/privacy}","received_events_url":"https://api.github.com/users/prateek/received_events","type":"User","site_admin":false},"body":"Don't you need to check if partial updates are allowed and insert those still? would be good to add a testcase for this ","created_at":"2018-04-25T15:43:06Z","updated_at":"2018-04-25T15:43:06Z","html_url":"https://github.com/m3db/m3ninx/pull/34#discussion_r184109376","pull_request_url":"https://api.github.com/repos/m3db/m3ninx/pulls/34","author_association":"COLLABORATOR","_links":{"self":{"href":"https://api.github.com/repos/m3db/m3ninx/pulls/comments/184109376"},"html":{"href":"https://github.com/m3db/m3ninx/pull/34#discussion_r184109376"},"pull_request":{"href":"https://api.github.com/repos/m3db/m3ninx/pulls/34"}}},"pull_request":{"url":"https://api.github.com/repos/m3db/m3ninx/pulls/34","id":183069933,"html_url":"https://github.com/m3db/m3ninx/pull/34","diff_url":"https://github.com/m3db/m3ninx/pull/34.diff","patch_url":"https://github.com/m3db/m3ninx/pull/34.patch","issue_url":"https://api.github.com/repos/m3db/m3ninx/issues/34","number":34,"state":"open","locked":false,"title":"[index] Add Batch method for inserting multiple documents at a time","user":{"login":"jeromefroe","id":7217598,"avatar_url":"https://avatars1.githubusercontent.com/u/7217598?v=4","gravatar_id":"","url":"https://api.github.com/users/jeromefroe","html_url":"https://github.com/jeromefroe","followers_url":"https://api.github.com/users/jeromefroe/followers","following_url":"https://api.github.com/users/jeromefroe/following{/other_user}","gists_url":"https://api.github.com/users/jeromefroe/gists{/gist_id}","starred_url":"https://api.github.com/users/jeromefroe/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jeromefroe/subscriptions","organizations_url":"https://api.github.com/users/jeromefroe/orgs","repos_url":"https://api.github.com/users/jeromefroe/repos","events_url":"https://api.github.com/users/jeromefroe/events{/privacy}","received_events_url":"https://api.github.com/users/jeromefroe/received_events","type":"User","site_admin":false},"body":"This PR adds a `Batch` method to the `index` interface for inserting a batch of documents at a time and adds a corresponding implementation to the in-memory `segment`.","created_at":"2018-04-20T13:55:08Z","updated_at":"2018-04-25T15:43:06Z","closed_at":null,"merged_at":null,"merge_commit_sha":"59a246bbf64e600a43d370b5e76914dd3c7075f7","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/m3db/m3ninx/pulls/34/commits","review_comments_url":"https://api.github.com/repos/m3db/m3ninx/pulls/34/comments","review_comment_url":"https://api.github.com/repos/m3db/m3ninx/pulls/comments{/number}","comments_url":"https://api.github.com/repos/m3db/m3ninx/issues/34/comments","statuses_url":"https://api.github.com/repos/m3db/m3ninx/statuses/55c2e2c2db9e6bd51de4077a4ae9e745c79db1db","head":{"label":"m3db:jeromefroe/support-batch-inserts","ref":"jeromefroe/support-batch-inserts","sha":"55c2e2c2db9e6bd51de4077a4ae9e745c79db1db","user":{"login":"m3db","id":19937171,"avatar_url":"https://avatars0.githubusercontent.com/u/19937171?v=4","gravatar_id":"","url":"https://api.github.com/users/m3db","html_url":"https://github.com/m3db","followers_url":"https://api.github.com/users/m3db/followers","following_url":"https://api.github.com/users/m3db/following{/other_user}","gists_url":"https://api.github.com/users/m3db/gists{/gist_id}","starred_url":"https://api.github.com/users/m3db/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/m3db/subscriptions","organizations_url":"https://api.github.com/users/m3db/orgs","repos_url":"https://api.github.com/users/m3db/repos","events_url":"https://api.github.com/users/m3db/events{/privacy}","received_events_url":"https://api.github.com/users/m3db/received_events","type":"Organization","site_admin":false},"repo":{"id":108438929,"name":"m3ninx","full_name":"m3db/m3ninx","owner":{"login":"m3db","id":19937171,"avatar_url":"https://avatars0.githubusercontent.com/u/19937171?v=4","gravatar_id":"","url":"https://api.github.com/users/m3db","html_url":"https://github.com/m3db","followers_url":"https://api.github.com/users/m3db/followers","following_url":"https://api.github.com/users/m3db/following{/other_user}","gists_url":"https://api.github.com/users/m3db/gists{/gist_id}","starred_url":"https://api.github.com/users/m3db/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/m3db/subscriptions","organizations_url":"https://api.github.com/users/m3db/orgs","repos_url":"https://api.github.com/users/m3db/repos","events_url":"https://api.github.com/users/m3db/events{/privacy}","received_events_url":"https://api.github.com/users/m3db/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/m3db/m3ninx","description":"Reverse index ","fork":false,"url":"https://api.github.com/repos/m3db/m3ninx","forks_url":"https://api.github.com/repos/m3db/m3ninx/forks","keys_url":"https://api.github.com/repos/m3db/m3ninx/keys{/key_id}","collaborators_url":"https://api.github.com/repos/m3db/m3ninx/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/m3db/m3ninx/teams","hooks_url":"https://api.github.com/repos/m3db/m3ninx/hooks","issue_events_url":"https://api.github.com/repos/m3db/m3ninx/issues/events{/number}","events_url":"https://api.github.com/repos/m3db/m3ninx/events","assignees_url":"https://api.github.com/repos/m3db/m3ninx/assignees{/user}","branches_url":"https://api.github.com/repos/m3db/m3ninx/branches{/branch}","tags_url":"https://api.github.com/repos/m3db/m3ninx/tags","blobs_url":"https://api.github.com/repos/m3db/m3ninx/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/m3db/m3ninx/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/m3db/m3ninx/git/refs{/sha}","trees_url":"https://api.github.com/repos/m3db/m3ninx/git/trees{/sha}","statuses_url":"https://api.github.com/repos/m3db/m3ninx/statuses/{sha}","languages_url":"https://api.github.com/repos/m3db/m3ninx/languages","stargazers_url":"https://api.github.com/repos/m3db/m3ninx/stargazers","contributors_url":"https://api.github.com/repos/m3db/m3ninx/contributors","subscribers_url":"https://api.github.com/repos/m3db/m3ninx/subscribers","subscription_url":"https://api.github.com/repos/m3db/m3ninx/subscription","commits_url":"https://api.github.com/repos/m3db/m3ninx/commits{/sha}","git_commits_url":"https://api.github.com/repos/m3db/m3ninx/git/commits{/sha}","comments_url":"https://api.github.com/repos/m3db/m3ninx/comments{/number}","issue_comment_url":"https://api.github.com/repos/m3db/m3ninx/issues/comments{/number}","contents_url":"https://api.github.com/repos/m3db/m3ninx/contents/{+path}","compare_url":"https://api.github.com/repos/m3db/m3ninx/compare/{base}...{head}","merges_url":"https://api.github.com/repos/m3db/m3ninx/merges","archive_url":"https://api.github.com/repos/m3db/m3ninx/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/m3db/m3ninx/downloads","issues_url":"https://api.github.com/repos/m3db/m3ninx/issues{/number}","pulls_url":"https://api.github.com/repos/m3db/m3ninx/pulls{/number}","milestones_url":"https://api.github.com/repos/m3db/m3ninx/milestones{/number}","notifications_url":"https://api.github.com/repos/m3db/m3ninx/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/m3db/m3ninx/labels{/name}","releases_url":"https://api.github.com/repos/m3db/m3ninx/releases{/id}","deployments_url":"https://api.github.com/repos/m3db/m3ninx/deployments","created_at":"2017-10-26T16:45:05Z","updated_at":"2018-04-23T18:03:54Z","pushed_at":"2018-04-24T22:45:38Z","git_url":"git://github.com/m3db/m3ninx.git","ssh_url":"git@github.com:m3db/m3ninx.git","clone_url":"https://github.com/m3db/m3ninx.git","svn_url":"https://github.com/m3db/m3ninx","homepage":null,"size":347,"stargazers_count":0,"watchers_count":0,"language":"Go","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":10,"license":{"key":"mit","name":"MIT License","spdx_id":"MIT","url":"https://api.github.com/licenses/mit"},"forks":0,"open_issues":10,"watchers":0,"default_branch":"master"}},"base":{"label":"m3db:master","ref":"master","sha":"96abb2b1b4f6ce1781177c67f2b95e0ee4eb8157","user":{"login":"m3db","id":19937171,"avatar_url":"https://avatars0.githubusercontent.com/u/19937171?v=4","gravatar_id":"","url":"https://api.github.com/users/m3db","html_url":"https://github.com/m3db","followers_url":"https://api.github.com/users/m3db/followers","following_url":"https://api.github.com/users/m3db/following{/other_user}","gists_url":"https://api.github.com/users/m3db/gists{/gist_id}","starred_url":"https://api.github.com/users/m3db/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/m3db/subscriptions","organizations_url":"https://api.github.com/users/m3db/orgs","repos_url":"https://api.github.com/users/m3db/repos","events_url":"https://api.github.com/users/m3db/events{/privacy}","received_events_url":"https://api.github.com/users/m3db/received_events","type":"Organization","site_admin":false},"repo":{"id":108438929,"name":"m3ninx","full_name":"m3db/m3ninx","owner":{"login":"m3db","id":19937171,"avatar_url":"https://avatars0.githubusercontent.com/u/19937171?v=4","gravatar_id":"","url":"https://api.github.com/users/m3db","html_url":"https://github.com/m3db","followers_url":"https://api.github.com/users/m3db/followers","following_url":"https://api.github.com/users/m3db/following{/other_user}","gists_url":"https://api.github.com/users/m3db/gists{/gist_id}","starred_url":"https://api.github.com/users/m3db/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/m3db/subscriptions","organizations_url":"https://api.github.com/users/m3db/orgs","repos_url":"https://api.github.com/users/m3db/repos","events_url":"https://api.github.com/users/m3db/events{/privacy}","received_events_url":"https://api.github.com/users/m3db/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/m3db/m3ninx","description":"Reverse index ","fork":false,"url":"https://api.github.com/repos/m3db/m3ninx","forks_url":"https://api.github.com/repos/m3db/m3ninx/forks","keys_url":"https://api.github.com/repos/m3db/m3ninx/keys{/key_id}","collaborators_url":"https://api.github.com/repos/m3db/m3ninx/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/m3db/m3ninx/teams","hooks_url":"https://api.github.com/repos/m3db/m3ninx/hooks","issue_events_url":"https://api.github.com/repos/m3db/m3ninx/issues/events{/number}","events_url":"https://api.github.com/repos/m3db/m3ninx/events","assignees_url":"https://api.github.com/repos/m3db/m3ninx/assignees{/user}","branches_url":"https://api.github.com/repos/m3db/m3ninx/branches{/branch}","tags_url":"https://api.github.com/repos/m3db/m3ninx/tags","blobs_url":"https://api.github.com/repos/m3db/m3ninx/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/m3db/m3ninx/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/m3db/m3ninx/git/refs{/sha}","trees_url":"https://api.github.com/repos/m3db/m3ninx/git/trees{/sha}","statuses_url":"https://api.github.com/repos/m3db/m3ninx/statuses/{sha}","languages_url":"https://api.github.com/repos/m3db/m3ninx/languages","stargazers_url":"https://api.github.com/repos/m3db/m3ninx/stargazers","contributors_url":"https://api.github.com/repos/m3db/m3ninx/contributors","subscribers_url":"https://api.github.com/repos/m3db/m3ninx/subscribers","subscription_url":"https://api.github.com/repos/m3db/m3ninx/subscription","commits_url":"https://api.github.com/repos/m3db/m3ninx/commits{/sha}","git_commits_url":"https://api.github.com/repos/m3db/m3ninx/git/commits{/sha}","comments_url":"https://api.github.com/repos/m3db/m3ninx/comments{/number}","issue_comment_url":"https://api.github.com/repos/m3db/m3ninx/issues/comments{/number}","contents_url":"https://api.github.com/repos/m3db/m3ninx/contents/{+path}","compare_url":"https://api.github.com/repos/m3db/m3ninx/compare/{base}...{head}","merges_url":"https://api.github.com/repos/m3db/m3ninx/merges","archive_url":"https://api.github.com/repos/m3db/m3ninx/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/m3db/m3ninx/downloads","issues_url":"https://api.github.com/repos/m3db/m3ninx/issues{/number}","pulls_url":"https://api.github.com/repos/m3db/m3ninx/pulls{/number}","milestones_url":"https://api.github.com/repos/m3db/m3ninx/milestones{/number}","notifications_url":"https://api.github.com/repos/m3db/m3ninx/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/m3db/m3ninx/labels{/name}","releases_url":"https://api.github.com/repos/m3db/m3ninx/releases{/id}","deployments_url":"https://api.github.com/repos/m3db/m3ninx/deployments","created_at":"2017-10-26T16:45:05Z","updated_at":"2018-04-23T18:03:54Z","pushed_at":"2018-04-24T22:45:38Z","git_url":"git://github.com/m3db/m3ninx.git","ssh_url":"git@github.com:m3db/m3ninx.git","clone_url":"https://github.com/m3db/m3ninx.git","svn_url":"https://github.com/m3db/m3ninx","homepage":null,"size":347,"stargazers_count":0,"watchers_count":0,"language":"Go","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":10,"license":{"key":"mit","name":"MIT License","spdx_id":"MIT","url":"https://api.github.com/licenses/mit"},"forks":0,"open_issues":10,"watchers":0,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/m3db/m3ninx/pulls/34"},"html":{"href":"https://github.com/m3db/m3ninx/pull/34"},"issue":{"href":"https://api.github.com/repos/m3db/m3ninx/issues/34"},"comments":{"href":"https://api.github.com/repos/m3db/m3ninx/issues/34/comments"},"review_comments":{"href":"https://api.github.com/repos/m3db/m3ninx/pulls/34/comments"},"review_comment":{"href":"https://api.github.com/repos/m3db/m3ninx/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/m3db/m3ninx/pulls/34/commits"},"statuses":{"href":"https://api.github.com/repos/m3db/m3ninx/statuses/55c2e2c2db9e6bd51de4077a4ae9e745c79db1db"}},"author_association":"CONTRIBUTOR"}}
{"actor":{"display_login":"prateek"}}