Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ip库增加mmdb格式支持 #653

Merged
merged 2 commits into from
Jul 26, 2018
Merged

Conversation

wangqiang-qiniu
Copy link
Contributor

Fixes [PDR-6915]

Changes

  • feature1
  • feature2
  • fixbug1
  • fixbug2

Reviewers

  • @wonderflow please review
  • @[someotherone] please review

Wiki Changes

  • options1...
  • options2...

Checklist

  • Rebased/mergeable
  • Tests pass
  • Wiki updated

@@ -141,3 +141,7 @@ func (loc *datLocator) Find(ipstr string) (info *LocationInfo, err error) {
}
return loc.FindByUint(binary.BigEndian.Uint32([]byte(ip.To4()))), nil
}

func (loc *datLocator) Close() error {
return nil
Copy link
Contributor

Choose a reason for hiding this comment

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

看看dat和datx要不要清理一下


func (loc *mmdbLocator) Close() error {
if loc.reader != nil {
locatorStore.Remove(loc.path)
Copy link
Contributor

Choose a reason for hiding this comment

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

这样直接remove可能不合适,因为别的tranformer或者runner也会用

@wonderflow wonderflow changed the title Ipmmdb Ip库增加mmdb格式支持 Jul 26, 2018
if err := loc.init(data); err != nil {
return nil, err
}
return loc, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

应该改造 newDatLocatorWithData 支持接收一个文件名参数

@@ -13,6 +13,7 @@ type datLocator struct {
indexData2 []int
indexData3 []int
index []int
path string
Copy link
Contributor

Choose a reason for hiding this comment

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

建议作为作为第一个字段


func (loc *datLocator) Close() error {
if loc.index != nil && len(loc.index) > 0 {
locatorStore.Remove(loc.path)
Copy link
Contributor

Choose a reason for hiding this comment

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

这里已经调用 Remove 之后,就没有地方引用这个 loc 了,会被 GC 收掉的,所以这行之后的内容应该是没有必要的

if err != nil {
loc := new(datxLocator)
loc.path = dataFile
if err := loc.init(data); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

应该改造 newDatxLocatorWithData 支持接收一个文件名参数


func (loc *datxLocator) Close() error {
if loc.index != nil && len(loc.index) > 0 {
locatorStore.Remove(loc.path)
Copy link
Contributor

Choose a reason for hiding this comment

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

这里已经调用 Remove 之后,就没有地方引用这个 loc 了,会被 GC 收掉的,所以这行之后的内容应该是没有必要的

assert.Len(t, locatorStore.locators, 2)
assert.Equal(t, expe6, ipt.stats)
assert.Equal(t, ipt.Stage(), transforms.StageAfterParser)
// 确保多个 transformer 只有两个 Locator 产生
Copy link
Contributor

Choose a reason for hiding this comment

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

这行注释的位置不对吧,原来判断是紧接着上一部分测试结果的,新添加的部分可以重新判断但是原来的还是不要移动

}

// 将指定路径删除
func (s *LocatorStore) Remove(fpath string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

// Remove 将指定路径的 Locator 引用计数减 1 并在为零时从 Store 中移除

locators map[string]Locator
lock sync.RWMutex
locators map[string]Locator
refCounts map[string]int
}

// Get 返回对应路径的 Locator,如果 Locator 不存在则返回 nil
func (s *LocatorStore) Get(fpath string) Locator {
Copy link
Contributor

Choose a reason for hiding this comment

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

// Get 返回对应路径的 Locator 并将其引用计数加 1,如果 Locator 不存在则返回 nil

type Closer interface {
Close() error
}

Copy link
Contributor

Choose a reason for hiding this comment

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

其实这个接口就是 io.Closer,不用新定义

@wonderflow
Copy link
Contributor

LGTM

@wonderflow wonderflow merged commit f466c4d into qiniu:master Jul 26, 2018
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.

3 participants