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.

Sign up for GitHub

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

Fix fetch record from master failed #2848

Merged
merged 3 commits into from
Jul 17, 2017
Merged

Fix fetch record from master failed #2848

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3f09194
fetch record from master
Yancey1989 Jul 13, 2017
d84e96a
dumps with protocol 0
Yancey1989 Jul 14, 2017
9f3be4e
merge freom develop branch
Yancey1989 Jul 17, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions go/cmd/master/master.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/namsral/flag"
log "github.com/sirupsen/logrus"
"github.com/topicai/candy"

"github.com/PaddlePaddle/Paddle/go/master"
"github.com/PaddlePaddle/Paddle/go/utils/networkhelper"
Expand All @@ -20,11 +21,18 @@ func main() {
port := flag.Int("port", 8080, "port of the master server.")
ttlSec := flag.Int("ttl", 60, "etcd lease TTL in seconds.")
endpoints := flag.String("endpoints", "http://127.0.0.1:2379", "comma separated etcd endpoints. If empty, fault tolerance will not be enabled.")
taskTimeoutDur := flag.Duration("task_timout_dur", 20*time.Minute, "task timout duration.")
taskTimeoutMax := flag.Int("task_timeout_max", 3, "max timtout count for each task before it being declared failed task.")
chunkPerTask := flag.Int("chunk_per_task", 10, "chunk per task.")
taskTimeoutDur := flag.Duration("task-timout-dur", 20*time.Minute, "task timout duration.")
taskTimeoutMax := flag.Int("task-timeout-max", 3, "max timtout count for each task before it being declared failed task.")
chunkPerTask := flag.Int("chunk-per-task", 10, "chunk per task.")
logLevel := flag.String("log-level", "info",
"log level, possible values: debug, info, warning, error, fatal, panic")
flag.Parse()

level, e := log.ParseLevel(*logLevel)
candy.Must(e)

log.SetLevel(level)

if *endpoints == "" {
log.Warningln("-endpoints not set, fault tolerance not be enabled.")
}
Expand Down
4 changes: 3 additions & 1 deletion go/master/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package master

import (
"os"
"time"

"github.com/PaddlePaddle/Paddle/go/connection"
"github.com/PaddlePaddle/recordio"
Expand Down Expand Up @@ -38,7 +39,8 @@ func (c *Client) getRecords() {
if err != nil {
// TODO(helin): wait before move on with next
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remove the TODO? since it's already done.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

// getTask call.
log.Errorln(err)
log.Errorf("Get task failed, sleep 3 seconds and continue, %s", err)
time.Sleep(3 * time.Second)
continue
}

Expand Down
3 changes: 1 addition & 2 deletions python/paddle/v2/dataset/common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,7 @@ def close_writers(w):
def write_data(w, lines):
random.shuffle(lines)
for i, d in enumerate(lines):
d = pickle.dumps(d, pickle.HIGHEST_PROTOCOL)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I modify the protocol version from HIGHEST_PROTOCOL to 0, because this will cause an EOFError:

Traceback (most recent call last):
  File "decode.py", line 13, in <module>
    print pickle.loads(o)
EOFError

I think this error maybe caused by RecordIO library: Use binary stream as input will caused an EOFError, the following is my test code, it will throw an EOFError

w = recordio.writer("tmp-recordio")
for i in xrange(10):
    w.write(pickle.dumps(i, pickle.HIGHEST_PROTOCOL))
w.close()

r = recordio.reader("tmp-recordio")
for i in xrange(10):
    o = r.read()
    print pickle.loads(o)
r.close()
=========
Traceback (most recent call last):
  File "decode.py", line 13, in <module>
    print pickle.loads(o)
EOFError

I will follow this problem, and maybe do not block this fix:)

Some documents:

Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/PaddlePaddle/recordio/blob/master/python/recordio/recordio_test.py#L9
这个单测确实没有设置协议。
不过0的编码后文件比较大，不如用pickle.HIGHEST_PROTOCOL或者负数。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

参考上面的comment，如果用pickle.HIGHEST_PROTOCOL 会导致RecordIO读取数据失败，感觉是RecordIO的library对二进制的输入支持的有些问题，我还在跟进，会在其他PR中修复这个问题。

w[i % num_shards].write(d)
w[i % num_shards].write(str(d))
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it hurt the speed if we remove the Pickle module?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am curious as well, why this change could fix the failure?

Copy link
Contributor

@gongweibao gongweibao Jul 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

recordio模块是不是没有更新？这个错误在我写convert的时候已经碰到过了，helin也已经fix掉了。
去掉Pickle解决这个问题，有点奇怪。。。

Copy link
Contributor

Choose a reason for hiding this comment

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

Curious why we need pickle? since the master is parsing recordio files directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The same to @typhoonzero , the master need recordio files, and the convert function write a file with pickle.

Copy link
Contributor

Choose a reason for hiding this comment

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

用户写入的可能是一个object，比如list，用str是无法完成序列化的吧?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks all! I got it and fixed.
Done.


w = open_writers()
lines = []
Expand Down