Skip to content
This repository was archived by the owner on Feb 13, 2022. It is now read-only.

Fixed bug when 'END' is contained in the cached data - #23

Closed
lusentis wants to merge 1 commit into
elbart:masterfrom
lusentis:master
Closed

Fixed bug when 'END' is contained in the cached data#23
lusentis wants to merge 1 commit into
elbart:masterfrom
lusentis:master

Conversation

@lusentis

Copy link
Copy Markdown

I have a problem getting cached data if it contains the string "END". I have fixed the problem by replacing .indexOf('END') with .lastIndexOf('END') as suggested in #22

@elbart

Copy link
Copy Markdown
Owner

i understand your point but with your fix most of the tests are failing... currently investigating on this problem... i hope to pull this asap since this is surely very useful...

@guileen

Copy link
Copy Markdown

same problem.

@1999

1999 commented Jan 17, 2013

Copy link
Copy Markdown

@elbart any changes on this? i left a comment in #26 regarding this. I hope it helps

@sricc

Copy link
Copy Markdown

Caught this problem as well. Any chance this will be fixed soon?

@masonzhang

Copy link
Copy Markdown

@elbart Please merge this pull request. This fixed my issue.

@1999

1999 commented Aug 4, 2013

Copy link
Copy Markdown

@masonzhang this pull request does not solve the problem completely. Look this comment #26 (comment) and you'll see that. As far as I understand @elbart discontinued the developing of node-memcache project and it's probably better to fork it now.

@masonzhang

Copy link
Copy Markdown

@1999 right, the fix cannot resolve the issue. After long run on server, I saw other error logs. Thanks a lot for the reminder. To further investigate the issue. I output the content of the cache to logs when error happens. Finally I found the reason is, sometimes the string read from cache may contains multiple cached object. The simple fix does not resolve the issue. I didn't read the document of memcached, but I think it should be documented in server protocol.

@masonzhangmasonzhang mentioned this pull request Oct 10, 2013
Firer added a commit to Firer/node-memcache that referenced this pull request Sep 13, 2018
@jisqyv

Copy link
Copy Markdown

@lusentis See #46 for a different (better) approach.

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@lusentis@elbart@guileen@1999@sricc@masonzhang@jisqyv