Compress only metadata by default (fixes #1374)

Godeps/Godeps.json

@@ -31,7 +31,7 @@
 		},
 		{
 			"ImportPath": "github.com/syncthing/protocol",
-			"Rev": "cd0cce4195105cefab80fce84664188b614032e8"
+			"Rev": "108b4e2e104610bdf416f2f156f35ee769276caf"
 		},
 		{
 			"ImportPath": "github.com/syndtr/goleveldb/leveldb",

Godeps/_workspace/src/github.com/syncthing/protocol/compression.go

@@ -0,0 +1,53 @@
+// Copyright (C) 2015 The Protocol Authors.
+
+package protocol
+
+import "fmt"
+
+type Compression int
+
+const (
+	CompressMetadata Compression = iota // zero value is the default, default should be "metadata"
+	CompressNever
+	CompressAlways
+
+	compressionThreshold = 128 // don't bother compressing messages smaller than this many bytes
+)
+
+var compressionMarshal = map[Compression]string{
+	CompressNever:    "never",
+	CompressMetadata: "metadata",
+	CompressAlways:   "always",
+}
+
+var compressionUnmarshal = map[string]Compression{
+	// Legacy
+	"false": CompressNever,
+	"true":  CompressMetadata,
+
+	// Current
+	"never":    CompressNever,
+	"metadata": CompressMetadata,
+	"always":   CompressAlways,
+}
+
+func (c Compression) String() string {
+	s, ok := compressionMarshal[c]
+	if !ok {
+		return fmt.Sprintf("unknown:%d", c)
+	}
+	return s
+}
+
+func (c Compression) GoString() string {
+	return fmt.Sprintf("%q", c.String())
+}
+
+func (c Compression) MarshalText() ([]byte, error) {
+	return []byte(compressionMarshal[c]), nil
+}
+
+func (c *Compression) UnmarshalText(bs []byte) error {
+	*c = compressionUnmarshal[string(bs)]
+	return nil
+}

Godeps/_workspace/src/github.com/syncthing/protocol/compression_test.go

@@ -0,0 +1,51 @@
+// Copyright (C) 2015 The Protocol Authors.
+
+package protocol
+
+import "testing"
+
+func TestCompressionMarshal(t *testing.T) {
+	uTestcases := []struct {
+		s string
+		c Compression
+	}{
+		{"true", CompressMetadata},
+		{"false", CompressNever},
+		{"never", CompressNever},
+		{"metadata", CompressMetadata},
+		{"filedata", CompressFiledata},
+		{"always", CompressAlways},
+		{"whatever", CompressNever},
+	}
+
+	mTestcases := []struct {
+		s string
+		c Compression
+	}{
+		{"never", CompressNever},
+		{"metadata", CompressMetadata},
+		{"filedata", CompressFiledata},
+		{"always", CompressAlways},
+	}
+
+	var c Compression
+	for _, tc := range uTestcases {
+		err := c.UnmarshalText([]byte(tc.s))
+		if err != nil {
+			t.Error(err)
+		}
+		if c != tc.c {
+			t.Errorf("%s unmarshalled to %d, not %d", tc.s, c, tc.c)
+		}
+	}
+
+	for _, tc := range mTestcases {
+		bs, err := tc.c.MarshalText()
+		if err != nil {
+			t.Error(err)
+		}
+		if s := string(bs); s != tc.s {
+			t.Errorf("%d marshalled to %q, not %q", tc.c, s, tc.s)
+		}
+	}
+}

Godeps/_workspace/src/github.com/syncthing/protocol/protocol.go

@@ -106,7 +106,7 @@ type rawConnection struct {
 	closed chan struct{}
 	once   sync.Once
 
-	compressionThreshold int // compress messages larger than this many bytes
+	compression Compression
 
 	rdbuf0 []byte // used & reused by readMessage
 	rdbuf1 []byte // used & reused by readMessage
@@ -135,25 +135,21 @@ const (
 	pingIdleTime = 60 * time.Second
 )
 
-func NewConnection(deviceID DeviceID, reader io.Reader, writer io.Writer, receiver Model, name string, compress bool) Connection {
+func NewConnection(deviceID DeviceID, reader io.Reader, writer io.Writer, receiver Model, name string, compress Compression) Connection {
 	cr := &countingReader{Reader: reader}
 	cw := &countingWriter{Writer: writer}
 
-	compThres := 1<<31 - 1 // compression disabled
-	if compress {
-		compThres = 128 // compress messages that are 128 bytes long or larger
-	}
 	c := rawConnection{
-		id:                   deviceID,
+		id:          deviceID,
-		name:                 name,
+		name:        name,
-		receiver:             nativeModel{receiver},
+		receiver:    nativeModel{receiver},
-		state:                stateInitial,
+		state:       stateInitial,
-		cr:                   cr,
+		cr:          cr,
-		cw:                   cw,
+		cw:          cw,
-		outbox:               make(chan hdrMsg),
+		outbox:      make(chan hdrMsg),
-		nextID:               make(chan int),
+		nextID:      make(chan int),
-		closed:               make(chan struct{}),
+		closed:      make(chan struct{}),
-		compressionThreshold: compThres,
+		compression: compress,
 	}
 
 	go c.readerLoop()
@@ -571,7 +567,15 @@ func (c *rawConnection) writerLoop() {
 					return
 				}
 
-				if len(uncBuf) >= c.compressionThreshold {
+				compress := false
+				switch c.compression {
+				case CompressAlways:
+					compress = true
+				case CompressMetadata:
+					compress = hm.hdr.msgType != messageTypeResponse
+				}
+
+				if compress && len(uncBuf) >= compressionThreshold {
 					// Use compression for large messages
 					hm.hdr.compression = true
 

Godeps/_workspace/src/github.com/syncthing/protocol/protocol_test.go

@@ -67,8 +67,8 @@ func TestPing(t *testing.T) {
 	ar, aw := io.Pipe()
 	br, bw := io.Pipe()
 
-	c0 := NewConnection(c0ID, ar, bw, nil, "name", true).(wireFormatConnection).next.(*rawConnection)
+	c0 := NewConnection(c0ID, ar, bw, nil, "name", CompressAlways).(wireFormatConnection).next.(*rawConnection)
-	c1 := NewConnection(c1ID, br, aw, nil, "name", true).(wireFormatConnection).next.(*rawConnection)
+	c1 := NewConnection(c1ID, br, aw, nil, "name", CompressAlways).(wireFormatConnection).next.(*rawConnection)
 
 	if ok := c0.ping(); !ok {
 		t.Error("c0 ping failed")
@@ -91,8 +91,8 @@ func TestPingErr(t *testing.T) {
 			eaw := &ErrPipe{PipeWriter: *aw, max: i, err: e}
 			ebw := &ErrPipe{PipeWriter: *bw, max: j, err: e}
 
-			c0 := NewConnection(c0ID, ar, ebw, m0, "name", true).(wireFormatConnection).next.(*rawConnection)
+			c0 := NewConnection(c0ID, ar, ebw, m0, "name", CompressAlways).(wireFormatConnection).next.(*rawConnection)
-			NewConnection(c1ID, br, eaw, m1, "name", true)
+			NewConnection(c1ID, br, eaw, m1, "name", CompressAlways)
 
 			res := c0.ping()
 			if (i < 8 || j < 8) && res {
@@ -167,8 +167,8 @@ func TestVersionErr(t *testing.T) {
 	ar, aw := io.Pipe()
 	br, bw := io.Pipe()
 
-	c0 := NewConnection(c0ID, ar, bw, m0, "name", true).(wireFormatConnection).next.(*rawConnection)
+	c0 := NewConnection(c0ID, ar, bw, m0, "name", CompressAlways).(wireFormatConnection).next.(*rawConnection)
-	NewConnection(c1ID, br, aw, m1, "name", true)
+	NewConnection(c1ID, br, aw, m1, "name", CompressAlways)
 
 	w := xdr.NewWriter(c0.cw)
 	w.WriteUint32(encodeHeader(header{
@@ -190,8 +190,8 @@ func TestTypeErr(t *testing.T) {
 	ar, aw := io.Pipe()
 	br, bw := io.Pipe()
 
-	c0 := NewConnection(c0ID, ar, bw, m0, "name", true).(wireFormatConnection).next.(*rawConnection)
+	c0 := NewConnection(c0ID, ar, bw, m0, "name", CompressAlways).(wireFormatConnection).next.(*rawConnection)
-	NewConnection(c1ID, br, aw, m1, "name", true)
+	NewConnection(c1ID, br, aw, m1, "name", CompressAlways)
 
 	w := xdr.NewWriter(c0.cw)
 	w.WriteUint32(encodeHeader(header{
@@ -213,8 +213,8 @@ func TestClose(t *testing.T) {
 	ar, aw := io.Pipe()
 	br, bw := io.Pipe()
 
-	c0 := NewConnection(c0ID, ar, bw, m0, "name", true).(wireFormatConnection).next.(*rawConnection)
+	c0 := NewConnection(c0ID, ar, bw, m0, "name", CompressAlways).(wireFormatConnection).next.(*rawConnection)
-	NewConnection(c1ID, br, aw, m1, "name", true)
+	NewConnection(c1ID, br, aw, m1, "name", CompressAlways)
 
 	c0.close(nil)
 

gui/assets/lang/lang-en.json

@@ -7,6 +7,7 @@
    "Add new folder?": "Add new folder?",
    "Address": "Address",
    "Addresses": "Addresses",
+   "All Data": "All Data",
    "Allow Anonymous Usage Reporting?": "Allow Anonymous Usage Reporting?",
    "Anonymous Usage Reporting": "Anonymous Usage Reporting",
    "Any devices configured on an introducer device will be added to this device as well.": "Any devices configured on an introducer device will be added to this device as well.",
@@ -16,6 +17,7 @@
    "Changelog": "Changelog",
    "Close": "Close",
    "Comment, when used at the start of a line": "Comment, when used at the start of a line",
+   "Compression": "Compression",
    "Compression is recommended in most setups.": "Compression is recommended in most setups.",
    "Connection Error": "Connection Error",
    "Copied from elsewhere": "Copied from elsewhere",
@@ -71,6 +73,7 @@
    "Local Discovery": "Local Discovery",
    "Local State": "Local State",
    "Maximum Age": "Maximum Age",
+   "Metadata Only": "Metadata Only",
    "Move to top of queue": "Move to top of queue",
    "Multi level wildcard (matches multiple directory levels)": "Multi level wildcard (matches multiple directory levels)",
    "Never": "Never",
@@ -80,6 +83,7 @@
    "No File Versioning": "No File Versioning",
    "Notice": "Notice",
    "OK": "OK",
+   "Off": "Off",
    "Offline": "Offline",
    "Online": "Online",
    "Out Of Sync": "Out Of Sync",

gui/index.html

@@ -370,9 +370,12 @@
                       <th><span class="glyphicon glyphicon-link"></span>&emsp;<span translate>Address</span></th>
                       <td class="text-right">{{deviceAddr(deviceCfg)}}</td>
                     </tr>
-                    <tr ng-if="!deviceCfg.Compression">
+                    <tr ng-if="deviceCfg.Compression != 'metadata'">
-                      <th><span class="glyphicon glyphicon-compressed"></span>&emsp;<span translate>Use Compression</span></th>
+                      <th><span class="glyphicon glyphicon-compressed"></span>&emsp;<span translate>Compression</span></th>
-                      <td translate class="text-right">No</td>
+                      <td class="text-right">
+                        <span ng-if="deviceCfg.Compression == 'always'" translate>All Data</span>
+                        <span ng-if="deviceCfg.Compression == 'never'" translate>Off</span>
+                      </td>
                     </tr>
                     <tr ng-if="deviceCfg.Introducer">
                       <th><span class="glyphicon glyphicon-thumbs-up"></span>&emsp;<span translate>Introducer</span></th>
@@ -502,12 +505,12 @@
               <p translate class="help-block">Enter comma separated "ip:port" addresses or "dynamic" to perform automatic discovery of the address.</p>
             </div>
             <div ng-if="!editingSelf" class="form-group">
-              <div class="checkbox">
+              <label translate>Compression</label>
-                <label>
+              <select class="form-control" ng-model="currentDevice.Compression">
-                  <input type="checkbox" ng-model="currentDevice.Compression"> <span translate>Use Compression</span>
+                <option value="always" translate>All Data</option>
-                </label>
+                <option value="metadata" translate>Metadata Only</option>
-                <p translate class="help-block">Compression is recommended in most setups.</p>
+                <option value="never" translate>Off</option>
-              </div>
+              </select>
             </div>
             <div ng-if="!editingSelf" class="form-group">
               <div class="checkbox">

gui/scripts/syncthing/core/controllers/syncthingController.js

@@ -713,7 +713,7 @@ angular.module('syncthing.core')
                 .then(function () {
                     $scope.currentDevice = {
                         AddressesStr: 'dynamic',
-                        Compression: true,
+                        Compression: 'metadata',
                         Introducer: false,
                         selectedFolders: {}
                     };
@@ -758,7 +758,7 @@ angular.module('syncthing.core')
             var deviceCfg = {
                 DeviceID: device,
                 AddressesStr: 'dynamic',
-                Compression: true,
+                Compression: 'metadata',
                 Introducer: false,
                 selectedFolders: {}
             };

internal/config/config.go

@@ -36,7 +36,7 @@ import (
 
 var l = logger.DefaultLogger
 
-const CurrentVersion = 8
+const CurrentVersion = 9
 
 type Configuration struct {
 	Version        int                   `xml:"version,attr"`
@@ -146,12 +146,12 @@ func (c *VersioningConfiguration) UnmarshalXML(d *xml.Decoder, start xml.StartEl
 }
 
 type DeviceConfiguration struct {
-	DeviceID    protocol.DeviceID `xml:"id,attr"`
+	DeviceID    protocol.DeviceID    `xml:"id,attr"`
-	Name        string            `xml:"name,attr,omitempty"`
+	Name        string               `xml:"name,attr,omitempty"`
-	Addresses   []string          `xml:"address,omitempty"`
+	Addresses   []string             `xml:"address,omitempty"`
-	Compression bool              `xml:"compression,attr"`
+	Compression protocol.Compression `xml:"compression,attr"`
-	CertName    string            `xml:"certName,attr,omitempty"`
+	CertName    string               `xml:"certName,attr,omitempty"`
-	Introducer  bool              `xml:"introducer,attr"`
+	Introducer  bool                 `xml:"introducer,attr"`
 }
 
 type FolderDeviceConfiguration struct {
@@ -320,6 +320,9 @@ func (cfg *Configuration) prepare(myID protocol.DeviceID) {
 	if cfg.Version == 7 {
 		convertV7V8(cfg)
 	}
+	if cfg.Version == 8 {
+		convertV8V9(cfg)
+	}
 
 	// Hash old cleartext passwords
 	if len(cfg.GUI.Password) > 0 && cfg.GUI.Password[0] != '$' {
@@ -411,6 +414,13 @@ func ChangeRequiresRestart(from, to Configuration) bool {
 	return false
 }
 
+func convertV8V9(cfg *Configuration) {
+	// Compression is interpreted and serialized differently, but no enforced
+	// changes. Still need a new version number since the compression stuff
+	// isn't understandable by earlier versions.
+	cfg.Version = 9
+}
+
 func convertV7V8(cfg *Configuration) {
 	// Add IPv6 announce server
 	if len(cfg.Options.GlobalAnnServers) == 1 && cfg.Options.GlobalAnnServers[0] == "udp4://announce.syncthing.net:22026" {
@@ -498,7 +508,7 @@ func convertV2V3(cfg *Configuration) {
 	// compression on all existing new. New devices will get compression on by
 	// default by the GUI.
 	for i := range cfg.Deprecated_Nodes {
-		cfg.Deprecated_Nodes[i].Compression = true
+		cfg.Deprecated_Nodes[i].Compression = protocol.CompressMetadata
 	}
 
 	// The global discovery format and port number changed in v0.9. Having the

internal/config/config_test.go

@@ -99,13 +99,13 @@ func TestDeviceConfig(t *testing.T) {
 				DeviceID:    device1,
 				Name:        "node one",
 				Addresses:   []string{"a"},
-				Compression: true,
+				Compression: protocol.CompressMetadata,
 			},
 			{
 				DeviceID:    device4,
 				Name:        "node two",
 				Addresses:   []string{"b"},
-				Compression: true,
+				Compression: protocol.CompressMetadata,
 			},
 		}
 		expectedDeviceIDs := []protocol.DeviceID{device1, device4}
@@ -176,24 +176,22 @@ func TestDeviceAddressesDynamic(t *testing.T) {
 	name, _ := os.Hostname()
 	expected := map[protocol.DeviceID]DeviceConfiguration{
 		device1: {
-			DeviceID:    device1,
+			DeviceID:  device1,
-			Addresses:   []string{"dynamic"},
+			Addresses: []string{"dynamic"},
-			Compression: true,
 		},
 		device2: {
-			DeviceID:    device2,
+			DeviceID:  device2,
-			Addresses:   []string{"dynamic"},
+			Addresses: []string{"dynamic"},
-			Compression: true,
 		},
 		device3: {
-			DeviceID:    device3,
+			DeviceID:  device3,
-			Addresses:   []string{"dynamic"},
+			Addresses: []string{"dynamic"},
-			Compression: true,
 		},
 		device4: {
-			DeviceID:  device4,
+			DeviceID:    device4,
-			Name:      name, // Set when auto created
+			Name:        name, // Set when auto created
-			Addresses: []string{"dynamic"},
+			Addresses:   []string{"dynamic"},
+			Compression: protocol.CompressMetadata,
 		},
 	}
 
@@ -208,6 +206,43 @@ func TestDeviceAddressesDynamic(t *testing.T) {
 	}
 }
 
+func TestDeviceCompression(t *testing.T) {
+	name, _ := os.Hostname()
+	expected := map[protocol.DeviceID]DeviceConfiguration{
+		device1: {
+			DeviceID:    device1,
+			Addresses:   []string{"dynamic"},
+			Compression: protocol.CompressMetadata,
+		},
+		device2: {
+			DeviceID:    device2,
+			Addresses:   []string{"dynamic"},
+			Compression: protocol.CompressMetadata,
+		},
+		device3: {
+			DeviceID:    device3,
+			Addresses:   []string{"dynamic"},
+			Compression: protocol.CompressNever,
+		},
+		device4: {
+			DeviceID:    device4,
+			Name:        name, // Set when auto created
+			Addresses:   []string{"dynamic"},
+			Compression: protocol.CompressMetadata,
+		},
+	}
+
+	cfg, err := Load("testdata/devicecompression.xml", device4)
+	if err != nil {
+		t.Error(err)
+	}
+
+	actual := cfg.Devices()
+	if !reflect.DeepEqual(actual, expected) {
+		t.Errorf("Devices differ;\n  E: %#v\n  A: %#v", expected, actual)
+	}
+}
+
 func TestDeviceAddressesStatic(t *testing.T) {
 	name, _ := os.Hostname()
 	expected := map[protocol.DeviceID]DeviceConfiguration{
@@ -224,9 +259,10 @@ func TestDeviceAddressesStatic(t *testing.T) {
 			Addresses: []string{"[2001:db8::44]:4444", "192.0.2.4:6090"},
 		},
 		device4: {
-			DeviceID:  device4,
+			DeviceID:    device4,
-			Name:      name, // Set when auto created
+			Name:        name, // Set when auto created
-			Addresses: []string{"dynamic"},
+			Addresses:   []string{"dynamic"},
+			Compression: protocol.CompressMetadata,
 		},
 	}
 

internal/config/testdata/devicecompression.xml

@@ -0,0 +1,8 @@
+<configuration version="5">
+    <device id="AIR6LPZ7K4PTTUXQSMUUCPQ5YWOEDFIIQJUG7772YQXXR5YD6AWQ" compression="true">
+    </device>
+    <device id="GYRZZQBIRNPV4T7TC52WEQYJ3TFDQW6MWDFLMU4SSSU6EMFBK2VA" compression="metadata">
+    </device>
+    <device id="LGFPDIT7SKNNJVJZA4FC7QNCRKCE753K72BW5QD2FOZ7FRFEP57Q" compression="false">
+    </device>
+</configuration>

internal/config/testdata/v9.xml

@@ -0,0 +1,12 @@
+<configuration version="9">
+    <folder id="test" path="testdata" ro="true" ignorePerms="false" rescanIntervalS="600">
+        <device id="AIR6LPZ-7K4PTTV-UXQSMUU-CPQ5YWH-OEDFIIQ-JUG777G-2YQXXR5-YD6AWQR"></device>
+        <device id="P56IOI7-MZJNU2Y-IQGDREY-DM2MGTI-MGL3BXN-PQ6W5BM-TBBZ4TJ-XZWICQ2"></device>
+    </folder>
+    <device id="AIR6LPZ-7K4PTTV-UXQSMUU-CPQ5YWH-OEDFIIQ-JUG777G-2YQXXR5-YD6AWQR" name="node one" compression="metadata">
+        <address>a</address>
+    </device>
+    <device id="P56IOI7-MZJNU2Y-IQGDREY-DM2MGTI-MGL3BXN-PQ6W5BM-TBBZ4TJ-XZWICQ2" name="node two" compression="metadata">
+        <address>b</address>
+    </device>
+</configuration>