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

elbart commented Jul 27, 2012

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

guileen commented Nov 25, 2012

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

sricc commented Jul 19, 2013

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.

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

jisqyv commented Dec 4, 2018

Copy link
Copy Markdown

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

@lusentis lusentis closed this May 27, 2020
Sign up for free to 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